code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Strict Equality Check Missing in _callOptionalReturn (Potential Issue with Success Evaluation) #226

Closed c4-bot-4 closed 5 months ago

c4-bot-4 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/TransferHub/CallOptionalReturn.sol#L12

Vulnerability details

Impact

The _callOptionalReturn function lacks a strict equality check when decoding the return data, which could lead to unexpected behavior if the called contract returns non-boolean values that are interpreted as true.

Without a strict equality check, the success of the external call may be incorrectly evaluated, leading to incorrect decisions based on the returned data.

Proof of Concept

contract MaliciousContract { // Malicious contract simulating a target contract function maliciousFunction() external pure returns (uint256) { // Simulate a contract function that returns a non-boolean value return 1; // Non-boolean value } }

contract VulnerableContract { event ExternalCallSuccess(bool success);

function _callOptionalReturn(address token, bytes memory data) internal returns (bool call) {
    (bool success, bytes memory returndata) = token.call(data);

    // Inadequate equality check that can misinterpret non-boolean return values as 'true'
    bool results = returndata.length == 0 || abi.decode(returndata, (bool));

    emit ExternalCallSuccess(success && results); // Emitting the result for demonstration

    // No strict equality check here, which may lead to unexpected behavior
    call = success && results && token.code.length > 0;
}

function testCallOptionalReturn(address token) external {
    // Calling the function with a contract address that returns a non-boolean value
    _callOptionalReturn(token, abi.encodeWithSignature("maliciousFunction()"));
}

}

Steps:

Deploy the MaliciousContract and VulnerableContract.
Call the testCallOptionalReturn function of VulnerableContract, passing the address of MaliciousContract.
Observe the emitted event ExternalCallSuccess.
Despite maliciousFunction returning a non-boolean value (1), the event may erroneously indicate success due to the missing strict equality check.

Tools Used

Manual review

Recommended Mitigation Steps

Implement a strict equality check when decoding the return data to ensure correct evaluation of the success of external calls. For example, compare the decoded value against the expected boolean value using 'require'

Assessed type

Other

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 5 months ago

Gpt

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid