From a0a9100e2fbcd8c8ba8160ae2b73c7d4374b0ddf Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 20 Nov 2023 21:03:17 +0100 Subject: [PATCH 1/3] fix: don't leak env parser error --- crates/cheatcodes/src/env.rs | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/crates/cheatcodes/src/env.rs b/crates/cheatcodes/src/env.rs index a0a458851ddb..60c1704ccfdf 100644 --- a/crates/cheatcodes/src/env.rs +++ b/crates/cheatcodes/src/env.rs @@ -236,7 +236,7 @@ impl Cheatcode for envOr_13Call { } fn env(key: &str, ty: &DynSolType) -> Result { - get_env(key).and_then(|val| string::parse(&val, ty).map_err(map_env_err(key))) + get_env(key).and_then(|val| string::parse(&val, ty).map_err(map_env_err(key, &val))) } fn env_default(key: &str, default: &T, ty: &DynSolType) -> Result { @@ -245,7 +245,7 @@ fn env_default(key: &str, default: &T, ty: &DynSolType) -> Result { fn env_array(key: &str, delim: &str, ty: &DynSolType) -> Result { get_env(key).and_then(|val| { - string::parse_array(val.split(delim).map(str::trim), ty).map_err(map_env_err(key)) + string::parse_array(val.split(delim).map(str::trim), ty).map_err(map_env_err(key, &val)) }) } @@ -263,9 +263,12 @@ fn get_env(key: &str) -> Result { } } -fn map_env_err(key: &str) -> impl FnOnce(Error) -> Error + '_ { +/// Converts the error message of a failed parsing attempt to a more user-friendly message that +/// doesn't leak the value. +fn map_env_err<'a>(key: &'a str, value: &'a str) -> impl FnOnce(Error) -> Error + 'a { move |e| { - let e = e.to_string(); + let e = e.to_string(); // failed parsing \"xy(123)\" as type `uint256`: parser error:\nxy(123)\n ^\nexpected at + // least one digit let mut e = e.as_str(); // cut off the message to not leak the value let sep = if let Some(idx) = e.rfind(" as type `") { @@ -274,6 +277,26 @@ fn map_env_err(key: &str) -> impl FnOnce(Error) -> Error + '_ { } else { ": " }; + // ensure we're also removing the value from the underlying alloy parser error message, See + // [alloy_dyn_abi::parser::Error::parser] + let e = e.replacen(&format!("\n{value}\n"), &format!("${key}"), 1); fmt_err!("failed parsing ${key}{sep}{e}") } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_env_uint() { + let key = "parse_env_uint"; + let value = "xy(123)"; + env::set_var(key, value); + + let err = env(key, &DynSolType::Uint(256)).unwrap_err().to_string(); + assert!(!err.contains(value)); + assert_eq!(err, "failed parsing $parse_env_uint as type `uint256`: parser error:$parse_env_uint ^\nexpected at least one digit"); + env::remove_var(key); + } +} From d4ba71878f4567ac2422b7c8cd292346e279c2be Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 20 Nov 2023 21:09:23 +0100 Subject: [PATCH 2/3] Update crates/cheatcodes/src/env.rs Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com> --- crates/cheatcodes/src/env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cheatcodes/src/env.rs b/crates/cheatcodes/src/env.rs index 60c1704ccfdf..78ea1c75019f 100644 --- a/crates/cheatcodes/src/env.rs +++ b/crates/cheatcodes/src/env.rs @@ -279,7 +279,7 @@ fn map_env_err<'a>(key: &'a str, value: &'a str) -> impl FnOnce(Error) -> Error }; // ensure we're also removing the value from the underlying alloy parser error message, See // [alloy_dyn_abi::parser::Error::parser] - let e = e.replacen(&format!("\n{value}\n"), &format!("${key}"), 1); + let e = e.replacen(&format!("\n{value}\n"), &format!("\n${key}\n"), 1); fmt_err!("failed parsing ${key}{sep}{e}") } } From 3d15dfd3ec64598feec483dfeeb77f752ea17019 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 20 Nov 2023 21:12:27 +0100 Subject: [PATCH 3/3] update test --- crates/cheatcodes/src/env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cheatcodes/src/env.rs b/crates/cheatcodes/src/env.rs index 78ea1c75019f..b4a4a9a0c1f9 100644 --- a/crates/cheatcodes/src/env.rs +++ b/crates/cheatcodes/src/env.rs @@ -296,7 +296,7 @@ mod tests { let err = env(key, &DynSolType::Uint(256)).unwrap_err().to_string(); assert!(!err.contains(value)); - assert_eq!(err, "failed parsing $parse_env_uint as type `uint256`: parser error:$parse_env_uint ^\nexpected at least one digit"); + assert_eq!(err, "failed parsing $parse_env_uint as type `uint256`: parser error:\n$parse_env_uint\n ^\nexpected at least one digit"); env::remove_var(key); } }