Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions massa-models/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,21 +217,23 @@ impl<'de> ::serde::Deserialize<'de> for Address {
impl FromStr for Address {
type Err = ModelsError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let err = Err(ModelsError::AddressParseError(s.to_string()));
let prefix_err = Err(ModelsError::AddressParseError(
"Invalid address: Address prefix 'AU' or 'AS' not found".to_string(),
));

// Handle the prefix ("A{U|S}")
let mut chars = s.chars();
let Some(ADDRESS_PREFIX) = chars.next() else {
return err;
return prefix_err;
};
let Some(pref) = chars.next() else {
return err;
return prefix_err;
};

let res = match pref {
'U' => Address::User(UserAddress::from_str_without_prefixed_type(chars.as_str())?),
'S' => Address::SC(SCAddress::from_str_without_prefixed_type(chars.as_str())?),
_ => return err,
_ => return prefix_err,
};
Ok(res)
}
Expand Down Expand Up @@ -683,7 +685,9 @@ mod test {

#[test]
fn test_address_errors() {
let expected_error_0 = "address parsing error: UnexpectedAddress".to_string();
let expected_error_0 =
"address parsing error: Invalid address: Address prefix 'AU' or 'AS' not found"
.to_string();
let actual_error_0 = Address::from_str("UnexpectedAddress")
.unwrap_err()
.to_string();
Expand Down
73 changes: 71 additions & 2 deletions massa-models/src/amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use massa_serialization::{U64VarIntDeserializer, U64VarIntSerializer};
use nom::error::{context, ContextError, ParseError};
use nom::{IResult, Parser};
use rust_decimal::prelude::*;
use serde::de::Unexpected;
use std::fmt;
use std::ops::Bound;
use std::str::FromStr;
Expand Down Expand Up @@ -414,7 +413,14 @@ impl<'de> serde::de::Visitor<'de> for AmountVisitor {
where
E: serde::de::Error,
{
Amount::from_str(value).map_err(|_| E::invalid_value(Unexpected::Str(value), &self))
// The parse error is propagated instead of reflecting the (attacker-controlled,
// possibly large) input via `Unexpected::Str(value)`: every message reachable from
// `Amount::from_str` is a fixed static string (rust_decimal's parse errors are
// `&'static str` literals, and the sign/precision/range checks use constant
// messages), so error-path allocation does not scale with the rejected field length.
// Matches the `map_err(E::custom)` pattern used by the other string-based
// deserializers (Address, Hash, PublicKey, Signature).
Comment thread
Leo-Besancon marked this conversation as resolved.
Amount::from_str(value).map_err(E::custom)
}

fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
Expand All @@ -433,3 +439,66 @@ impl serde::Serialize for Amount {
serializer.serialize_str(&self.to_string())
}
}

#[cfg(test)]
mod tests {
use super::Amount;
use std::str::FromStr;

#[test]
fn test_valid_amount_still_deserializes() {
let amount: Amount = serde_json::from_str("\"12.34\"").unwrap();
assert_eq!(amount, Amount::from_str("12.34").unwrap());
}

#[test]
fn test_invalid_amount_error_is_bounded_and_explanatory() {
// The parse error is propagated (short static description from the parser),
// but the rejected Amount string itself must never be embedded in the error
// message, to avoid error-path allocation scaling with the input length.
let bogus = "z".repeat(4096);
let json = format!("\"{}\"", bogus);
let err = serde_json::from_str::<Amount>(&json)
.expect_err("invalid amount must fail to deserialize");
let msg = err.to_string();
assert!(
!msg.contains('z'),
"error message must not reflect the rejected input"
);
assert!(msg.len() < 128, "error message should stay short: {msg}");
assert!(
msg.contains("Invalid decimal: unknown character"),
"error message should explain the failure: {msg}"
);
}

#[test]
fn test_amount_error_messages_are_propagated() {
// Negative amounts surface the Amount-level explanation.
let err = serde_json::from_str::<Amount>("\"-1.5\"")
.expect_err("negative amount must fail to deserialize")
.to_string();
assert!(
err.contains("amounts cannot be strictly negative"),
"unexpected error: {err}"
);

// Over-precise amounts surface the Amount-level explanation.
let err = serde_json::from_str::<Amount>("\"1.1234567891\"")
.expect_err("over-precise amount must fail to deserialize")
.to_string();
assert!(
err.contains("amounts cannot be more precise than"),
"unexpected error: {err}"
);

// Syntactically invalid decimals surface the rust_decimal explanation.
let err = serde_json::from_str::<Amount>("\"1.2.3\"")
.expect_err("invalid decimal must fail to deserialize")
.to_string();
assert!(
err.contains("Invalid decimal: two decimal points"),
"unexpected error: {err}"
);
}
}
2 changes: 1 addition & 1 deletion massa-models/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub enum ModelsError {
ThreadOverflowError,
/// period overflow error
PeriodOverflowError,
/// amount parse error
/// amount parse error: {0}
AmountParseError(String),
/// address parsing error: {0}
AddressParseError(String),
Expand Down
Loading