Skip to content

Commit

Permalink
Merge branch 'tomas/router-loop' (#3340)
Browse files Browse the repository at this point in the history
* up/tomas/router-loop:
  changelog: add #3340
  sdk/queries/vp/pos: sanitize the input of `validator_by_tm_addr`
  • Loading branch information
Fraccaman committed Jun 3, 2024
2 parents b356690 + b377eb1 commit 347d176
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/unreleased/bug-fixes/3340-validator--by-tm-addr.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Fixes an issue with unsanitized input to a PoS query to find
a validator by TM address which may cause a node to panic.
([\#3340](https://github.com/anoma/namada/pull/3340))
49 changes: 48 additions & 1 deletion crates/sdk/src/queries/vp/pos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use namada_proof_of_stake::types::{
Slash, ValidatorMetaData, WeightedValidator,
};
use namada_proof_of_stake::{bond_amount, query_reward_tokens};
use namada_state::{DBIter, StorageHasher, DB};
use namada_state::{DBIter, KeySeg, StorageHasher, DB};
use namada_storage::collections::lazy_map;
use namada_storage::OptionExt;

Expand Down Expand Up @@ -643,6 +643,13 @@ where
D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
H: 'static + StorageHasher + Sync,
{
// Sanitize the input to make sure it doesn't crash in
// `namada_proof_of_stake::storage_key::validator_address_raw_hash_key`
if namada_storage::DbKeySeg::parse(tm_addr.clone()).is_err() {
return Err(namada_storage::Error::new_const(
"Invalid Tendermint address",
));
}
namada_proof_of_stake::storage::find_validator_by_raw_hash(
ctx.state, tm_addr,
)
Expand Down Expand Up @@ -776,3 +783,43 @@ fn enrich_bonds_and_unbonds(
total_withdrawable,
})
}

#[cfg(test)]
mod test {
use super::*;
use crate::queries::testing::TestClient;
use crate::queries::{RequestCtx, RequestQuery, Router};

#[tokio::test]
async fn test_validator_by_tm_addr_sanitized_input() {
let client = TestClient::new(POS);

// Test request with an invalid path - the trailing slash ends up being
// part of the input where in `fn validator_by_tm_addr` the
// parameter will be:
// `tm_addr = "52894D2ABA1614EF24CC1DDAE127A7A2386DE3BB/"`
let request = RequestQuery {
path: "/validator_by_tm_addr/\
52894D2ABA1614EF24CC1DDAE127A7A2386DE3BB/"
.to_owned(),
data: Default::default(),
height: 0_u32.into(),
prove: Default::default(),
};
let ctx = RequestCtx {
event_log: &client.event_log,
state: &client.state,
vp_wasm_cache: (),
tx_wasm_cache: (),
storage_read_past_height_limit: None,
};
let result = POS.handle(ctx, &request);
assert!(result.is_err());
assert!(
result
.unwrap_err()
.to_string()
.contains("Invalid Tendermint address")
)
}
}

0 comments on commit 347d176

Please sign in to comment.