Skip to content

Commit

Permalink
fix: implement valid AccountId generation from arbitrary data (#9107)
Browse files Browse the repository at this point in the history
Looks like the former implementation of `Arbitrary` for `AccountId` would assume all strings were well-formed `AccountId`'s. Skipping the validation logic. Which is untrue.

This patch swaps out the derive for a custom implementation that ensures all arbitrary byte slices are validated before an `AccountId` is constructed.
  • Loading branch information
miraclx authored and nikurt committed Jun 13, 2023
1 parent c745421 commit 6106a72
Showing 1 changed file with 11 additions and 30 deletions.
41 changes: 11 additions & 30 deletions core/account-id/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,15 +377,18 @@ impl<'a> arbitrary::Arbitrary<'a> for AccountId {
}

fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
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;
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)
}
_ => break Err(arbitrary::Error::IncorrectFormat),
}
}
}
Expand Down Expand Up @@ -719,26 +722,4 @@ 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 6106a72

Please sign in to comment.