code-423n4 / 2023-01-opensea-findings

0 stars 0 forks source link

Lack of Input Validation: Unchecked Inputs Allowing for Malicious Data and Unexpected Behavior #100

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/helpers/PointerLibraries.sol#L1125

Vulnerability details

Impact

The contract does not perform any checks on the inputs passed to the functions, which could allow an attacker to pass malicious data and trigger unexpected behavior or errors.

function readInt208( ReturndataPointer rdPtr ) internal pure returns (int208 value) { assembly { returndatacopy(0, rdPtr, 0x20) value := mload(0) } }

This function reads the int208 at rdPtr in returndata. However, it does not perform any checks on the input rdPtr to ensure that it is within bounds. This could allow an attacker to pass a malicious rdPtr that exceeds the size of returndata, potentially leading to reading or writing arbitrary memory locations.

Proof of Concept

function exploit(ReturndataPointer rdPtr) external { int208 value = readInt208(rdPtr); }

An attacker can call the function exploit and pass an arbitrary value for rdPtr that exceeds the size of returndata, which will cause the readInt208 function to read or write arbitrary memory locations.

Tools Used

vs code

Recommended Mitigation Steps

implement input validation checks on the rdPtr input to ensure that it is within bounds before executing any sensitive operations.

0age commented 1 year ago

contested (looks to me like a duplicate)

hrkrshnn commented 1 year ago

This could allow an attacker to pass a malicious rdPtr that exceeds the size of returndata

Out of bounds returndata access (in returndatacopy) would revert.

Moreover, the memory region that this writes into is well-defined-- [0, 0x20). So, the following is never a concern, regardless of the out-of-bounds access reverting or not.

which will cause the readInt208 function to read or write arbitrary memory locations.

HickupHH3 commented 1 year ago

size is 1 word, destOffset = 0, so yeah, writing into scratch space [0, 0x20).

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Insufficient quality