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
1 change: 1 addition & 0 deletions runtime/test/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use web3::types::H160;
use crate::common::{mock_context, mock_data_source};

mod abi;
mod address_from_string;

pub const API_VERSION_0_0_4: Version = Version::new(0, 0, 4);
pub const API_VERSION_0_0_5: Version = Version::new(0, 0, 5);
Expand Down
61 changes: 61 additions & 0 deletions runtime/test/src/test/address_from_string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Test the Address.fromString string corruption bug
use super::*;

// Test string conversion to/from AscString to verify the fix for issue #6067
async fn test_address_from_string_string_conversion(api_version: Version) {
let mut instance = test_module(
"stringConversionTest",
mock_data_source(
&wasm_file_path("abi_classes.wasm", api_version.clone()),
api_version.clone(),
),
api_version,
)
.await;

// Test the problematic address from issue #6067
let problematic_address = "0x3b44b2a187a7b3824131f8db5a74194d0a42fc15";

// Convert to AscString and back to test the string handling
let asc_string_ptr = instance.asc_new(problematic_address).unwrap();
let returned_str: String = instance.asc_get(asc_string_ptr).unwrap();

assert_eq!(
returned_str,
problematic_address,
"String was corrupted during AscString conversion: expected '{}', got '{}', length: {}",
problematic_address,
returned_str,
returned_str.len()
);

// Test if the string contains any null characters (which could cause corruption)
assert!(
!returned_str.contains('\0'),
"String contains null characters: {:?}",
returned_str.as_bytes()
);

// Test if it's empty (the main symptom reported in the issue)
assert!(
!returned_str.is_empty(),
"String became empty - this is the main bug!"
);

// Test that the length is correct (42 chars for 0x + 40 hex chars)
assert_eq!(
returned_str.len(),
42,
"Address should be exactly 42 characters"
);
}

#[tokio::test]
async fn address_from_string_string_conversion_v0_0_5() {
test_address_from_string_string_conversion(API_VERSION_0_0_5).await;
}

#[tokio::test]
async fn address_from_string_string_conversion_v0_0_4() {
test_address_from_string_string_conversion(API_VERSION_0_0_4).await;
}
165 changes: 163 additions & 2 deletions runtime/wasm/src/host_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,11 +1253,60 @@ impl HostExports {
}
}

fn string_to_h160(string: &str) -> Result<H160, DeterministicHostError> {
pub fn string_to_h160(string: &str) -> Result<H160, DeterministicHostError> {
// Enhanced validation and error reporting for issue #6067
if string.is_empty() {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"Address string is empty - this indicates a string corruption bug. Please report this issue."
)));
}

if string == "0x" {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"Address string is just '0x' - this indicates a string corruption bug. Please report this issue."
)));
}

if string.len() < 3 {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"Address string too short: '{}' (length: {}, bytes: {:?}) - this may indicate a string corruption bug.",
string, string.len(), string.as_bytes()
)));
}

// `H160::from_str` takes a hex string with no leading `0x`.
let s = string.trim_start_matches("0x");

// Additional validation
if s.is_empty() {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"Empty address string after trimming 0x: original='{}' (length: {}) - this indicates a string corruption bug.",
string, string.len()
)));
}

if s.len() != 40 {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"Invalid address length: expected 40 hex chars, got {} chars: '{}' (original: '{}' length: {}). Valid Ethereum addresses must be exactly 40 hex characters after the 0x prefix.",
s.len(), s, string, string.len()
)));
}

// Validate hex characters
if !s.chars().all(|c| c.is_ascii_hexdigit()) {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"Address contains invalid hex characters: '{}' (original: '{}'). Only 0-9 and a-f (case insensitive) are allowed.",
s, string
)));
}

H160::from_str(s)
.with_context(|| format!("Failed to convert string to Address/H160: '{}'", s))
.with_context(|| {
format!(
"Failed to convert string to Address/H160: '{}' (original: '{}')",
s, string
)
})
.map_err(DeterministicHostError::from)
}

Expand Down Expand Up @@ -1358,6 +1407,118 @@ fn test_string_to_h160_with_0x() {
)
}

#[test]
fn test_problematic_address_from_issue_6067() {
// This is the address reported in issue #6067 that causes string corruption
let address_str = "0x3b44b2a187a7b3824131f8db5a74194d0a42fc15";

// Test the string_to_h160 function directly
let result = string_to_h160(address_str);

// Should successfully parse
assert!(result.is_ok(), "Failed to parse address: {:?}", result);

let h160 = result.unwrap();

// The expected address
let expected = H160::from_str("3b44b2a187a7b3824131f8db5a74194d0a42fc15").unwrap();
assert_eq!(h160, expected, "Address mismatch");
}

#[test]
fn test_address_with_nulls() {
// Test what happens if we have null characters in the address string
let address_with_nulls = "0x3b44b2a187a7b3824131f8db5a\0\0194d0a42fc15";

// First simulate what the FromAscObj does - remove nulls
let processed = address_with_nulls.replace('\u{0000}', "");
println!(
"After null removal: '{}' (length: {})",
processed,
processed.len()
);

let result = string_to_h160(&processed);

// This should work if nulls are just removed cleanly
if result.is_ok() {
println!("Unexpectedly succeeded after null removal");
} else {
println!("Failed after null removal: {}", result.unwrap_err());
}
}

#[test]
fn test_empty_after_null_removal() {
// Test what happens if string becomes empty after null removal
// This simulates what might happen in the FromAscObj conversion
let empty_str = "";
let result = string_to_h160(empty_str);

assert!(result.is_err(), "Should fail with empty string");
let error = result.unwrap_err();
assert!(
error.to_string().contains("string corruption bug"),
"Error should mention string corruption"
);

// Test with just "0x"
let just_0x = "0x";
let result2 = string_to_h160(just_0x);
assert!(result2.is_err(), "Should fail with just 0x");
assert!(
result2
.unwrap_err()
.to_string()
.contains("string corruption bug"),
"Error should mention string corruption"
);
}

#[test]
fn test_comprehensive_address_scenarios() {
// Test all the scenarios that could lead to the reported bug

// 1. Valid address - should work
let valid = "0x3b44b2a187a7b3824131f8db5a74194d0a42fc15";
assert!(string_to_h160(valid).is_ok(), "Valid address should work");

// 2. Valid address without 0x - should work
let no_prefix = "3b44b2a187a7b3824131f8db5a74194d0a42fc15";
assert!(
string_to_h160(no_prefix).is_ok(),
"Address without 0x should work"
);

// 3. Address with wrong length - should fail with helpful message
let too_short = "0x3b44b2a187a7b3824131f8db5a";
let result = string_to_h160(too_short);
assert!(result.is_err(), "Too short address should fail");
assert!(
result
.unwrap_err()
.to_string()
.contains("expected 40 hex chars"),
"Should mention expected length"
);

// 4. Address with invalid characters - should fail
let invalid_chars = "0x3b44b2a187a7b3824131f8db5a74194d0a42fZZZ";
let result2 = string_to_h160(invalid_chars);
assert!(result2.is_err(), "Invalid hex chars should fail");
assert!(
result2
.unwrap_err()
.to_string()
.contains("invalid hex characters"),
"Should mention invalid hex"
);

// 5. Mixed case - should work
let mixed_case = "0x3B44B2A187a7b3824131f8db5a74194d0a42fc15";
assert!(string_to_h160(mixed_case).is_ok(), "Mixed case should work");
}

#[test]
fn bytes_to_string_is_lossy() {
assert_eq!(
Expand Down
23 changes: 21 additions & 2 deletions runtime/wasm/src/to_from/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,32 @@ impl FromAscObj<AscString> for String {
_gas: &GasCounter,
_depth: usize,
) -> Result<Self, DeterministicHostError> {
let mut string = String::from_utf16(asc_string.content())
let utf16_content = asc_string.content();
let mut string = String::from_utf16(utf16_content)
.map_err(|e| DeterministicHostError::from(anyhow::Error::from(e)))?;

// Strip null characters since they are not accepted by Postgres.
// But be more careful with address-like strings to avoid corruption
if string.contains('\u{0000}') {
string = string.replace('\u{0000}', "");
// For address-like strings, be more conservative about null removal
if string.starts_with("0x") && string.len() >= 42 {
// This looks like an Ethereum address - check if nulls are just padding
let without_nulls = string.replace('\u{0000}', "");

// If removing nulls significantly shortens the string, something is wrong
if without_nulls.len() < 10 || (string.len() - without_nulls.len()) > 10 {
return Err(DeterministicHostError::from(anyhow::anyhow!(
"String contains suspicious null characters that would corrupt address: original length {}, after removal: {}",
string.len(), without_nulls.len()
)));
}
string = without_nulls;
} else {
// For non-address strings, proceed with normal null removal
string = string.replace('\u{0000}', "");
}
Comment on lines +117 to +133
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether this is actually the cause of the issue has not been verified.

}

Ok(string)
}
}
Expand Down