Skip to content

Commit

Permalink
chore: fix panic on arbitrary AccountId generation (#9124)
Browse files Browse the repository at this point in the history
Fixes unintended panic in the `Arbitrary` implementation of `AccountId`.

As pointed out in #9107 (comment), certain `Unstructured` data sources like `a|bcd` or `a_-b` causes a panic with the current implementation.

This patch reduces the consumed input to the limit of validity adherence. Failing otherwise. But never panics.

Worth noting this implementation does not provide a fallback for input sources shorter than `AccountId::MIN_LEN` or longer than `AccountId::MAX_LEN`.
  • Loading branch information
miraclx authored and Nikolay Kurtov committed Jun 9, 2023
1 parent a439c61 commit 9d57775
Showing 1 changed file with 30 additions and 11 deletions.
41 changes: 30 additions & 11 deletions core/account-id/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,18 +377,15 @@ impl<'a> arbitrary::Arbitrary<'a> for AccountId {
}

fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
let s = u.arbitrary::<&str>()?;
match s.parse::<AccountId>() {
Ok(account_id) => Ok(account_id),
Err(e) => {
if let Some((valid_up_to, _)) = e.char {
let valid = &s[..valid_up_to];
debug_assert!(AccountId::validate(valid).is_ok());
#[allow(deprecated)]
Ok(AccountId::new_unvalidated(valid.to_string()))
} else {
Err(arbitrary::Error::IncorrectFormat)
let mut s = u.arbitrary::<&str>()?;
loop {
match s.parse::<AccountId>() {
Ok(account_id) => break Ok(account_id),
Err(ParseAccountError { char: Some((idx, _)), .. }) => {
s = &s[..idx];
continue;
}
_ => break Err(arbitrary::Error::IncorrectFormat),
}
}
}
Expand Down Expand Up @@ -722,4 +719,26 @@ mod tests {
);
}
}

#[test]
#[cfg(feature = "arbitrary")]
fn test_arbitrary() {
let corpus = [
("a|bcd", None),
("ab|cde", Some("ab")),
("a_-b", None),
("ab_-c", Some("ab")),
("a", None),
("miraclx.near", Some("miraclx.near")),
("01234567890123456789012345678901234567890123456789012345678901234", None),
];

for (input, expected_output) in corpus {
assert!(input.len() <= u8::MAX as usize);
let data = [input.as_bytes(), &[input.len() as _]].concat();
let mut u = arbitrary::Unstructured::new(&data);

assert_eq!(u.arbitrary::<AccountId>().as_deref().ok(), expected_output);
}
}
}

0 comments on commit 9d57775

Please sign in to comment.