Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/leo/src/events.rs#L30

Vulnerability details

Summary:

The pack_details function in the smart contract code is designed to pack three fields—tick_lower, tick_upper, and owner (an Ethereum address)—into a single U256 value. However, there is a bug in the way the owner address is handled. The improper packing of the 160-bit address (U160) into a 256-bit unsigned integer (U256) can lead to data corruption or misalignment, causing the contract to behave unexpectedly when reading or processing packed data.

Vulnerability Detail:

In Ethereum, addresses are typically 160-bit values. The function pack_details takes an address as input, converts it into a U160, and then attempts to pack it into the lower 160 bits of a U256 variable. However, the code relies on an implicit cast from U160 to U256, which may not properly handle the lower 160 bits of the address. This can result in incorrect packing, data misalignment, or even truncation of address values.

The following code demonstrates the issue:

Code Snippet:

fn pack_details(tick_lower: i32, tick_upper: i32, owner: Address) -> U256 {
    let mut packed = U256::from(tick_lower as u32) << (32 + 160);
    packed |= U256::from(tick_upper as u32) << 160;
    let owner: U160 = owner.into();
    packed | U256::from(owner)  // Potential issue: implicit cast from U160 to U256
}

Impact:

Data corruption: The improperly packed U160 address could result in incorrect or truncated values when attempting to unpack and use the data.
Security vulnerabilities: If the address used in the contract is misaligned or truncated, it could lead to misidentification of the actual contract owner or cause incorrect transfers of ownership or funds.
Logic errors: This bug may cause the contract to behave in unpredictable ways, especially when interacting with external systems that rely on correctly packed data.

Technical Analysis:

Implicit cast issues: The U160 to U256 cast relies on default Rust behavior, which may not always ensure that the lower 160 bits of the U256 contain the address. This implicit conversion is risky, as it can lead to the higher bits being incorrectly populated or the lower bits being truncated.
Bit-shifting and misalignment: Without explicitly controlling the placement of the address bits, the code is vulnerable to misalignment of values, especially when combined with other data fields like tick_lower and tick_upper.

Recommendations:

Explicit Address Packing: Use bitwise operations to explicitly control the packing of the 160-bit address into the lower 160 bits of the U256, ensuring that the address is properly aligned.
Validate tick_lower and tick_upper: Ensure that tick_lower and tick_upper are non-negative before packing them into the U256. This avoids the potential issue of casting negative values to u32, which can lead to wrapping and unexpected behavior.
Unit Tests: Add tests that specifically check the packed and unpacked values for correctness. Ensure that edge cases like the minimum and maximum values for tick_lower and tick_upper are covered.
Avoid Implicit Casts: Explicitly define the conversion process for critical fields like addresses to avoid unexpected behavior.

Fixed Code Snippet:

fn pack_details(tick_lower: i32, tick_upper: i32, owner: Address) -> U256 {
    // Ensure tick values are within the valid range (handle negative values)
    if tick_lower < 0 || tick_upper < 0 {
        panic!("Ticks must be non-negative");
    }

    // Convert tick values to U256 and shift them into place
    let mut packed = U256::from(tick_lower as u32) << (32 + 160); // 32-bit tick_lower in the uppermost bits
    packed |= U256::from(tick_upper as u32) << 160;               // 32-bit tick_upper just below tick_lower

    // Explicitly ensure that the address fits into the lower 160 bits
    let owner: U160 = owner.into();
    packed |= U256::from(owner);                                  // Address explicitly packed into the lower 160 bits

    packed
}

Impact Summary:

By improperly packing the address and relying on implicit casting, the contract risks significant logic errors that could lead to:

Misidentification of users or contract owners.
Incorrectly handled ownership or token transfers.
Potential vulnerabilities that malicious actors could exploit to gain unauthorized control.

Conclusion:

The improper packing of addresses and reliance on implicit conversions in the pack_details function presents a significant risk of data corruption. By implementing explicit bitwise operations and properly testing edge cases, this issue can be mitigated, improving both the safety and reliability of the contract.

Assessed type

Invalid Validation

code-423n4 / 2024-08-superposition-validation

Improper Address Packing in pack_details Function Leading to Potential Data Corruption #198