Non-cleaned up variable in a inline assembly

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/d86e73034e6c9e81124cd1a763c3a7288268f1ab/src/Market.sol#L226 https://github.com/code-423n4/2022-10-inverse/blob/d86e73034e6c9e81124cd1a763c3a7288268f1ab/src/Market.sol#L292

Vulnerability details

Vulnerability details

Description

There are functions createEscrow and predictEscrow from Market contract. Both accept address user parameter and process it inside assembly.

        assembly {
            let ptr := mload(0x40)
            mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
            mstore(add(ptr, 0x14), shl(0x60, implementation))
            mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
            instance := create2(0, ptr, 0x37, user)
        }

So, user variable is used in an inline assembly and is a parameter in a create2 salt.

According to Solidity documentation, there is no guarantee that such variable will be cleaned up, so processing it in the assembly instruction can lead to a completely wrong result.

Even though I couldn't find a place where the user variable became non-cleaned up, it is clear that there is no explicit check about that.

PoC

// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity =0.8.7;

contract PoC {
    function standard_division(uint24 a, uint24 b) internal returns (uint24 result) {
        assembly {
            result := div(a, b)
        }
    }

    function test() external {
        uint256 a = 2**100 + 11;
        uint256 b = 2**80 + 3;

        uint24 a_as_uin24 = uint24(a);
        require(a_as_uin24 == 11);
        uint24 b_as_uin24 = uint24(b);
        require(b_as_uin24 == 3);

        require(standard_division(11, 3) == 3);
        require(standard_division(a_as_uin24, b_as_uin24) == uint24(1048575));
    }
}

Recommended Mitigation Steps

Add instructions that clean up the input variable which is supposed to store smaller than 256 bits of information before using it in the assembly construction.

[0xean]

Downgrade to QA

[c4-judge]

captainmangoC4 changed the severity to QA (Quality Assurance)

[c4-judge]

0xean marked the issue as grade-c