Assessment reports>Biconomy Batched Session Router Module>Informational findings>Add Length Validation for ,callData, in ,validateSessionUserOp
Category: Coding Mistakes

Add Length Validation for callData in validateSessionUserOp

Informational Severity
Informational Impact
N/A Likelihood

Description

In the validateSessionUserOp function, the length check for op.callData is incomplete, and there may be callData with illegal length.

    function validateSessionUserOp(
        UserOperation calldata _op,
        bytes32 _userOpHash,
        bytes calldata _sessionKeyData,
        bytes calldata _sessionKeySignature
    ) external pure override returns (bool) {
        
        ...

        // working with userOp.callData
        // check if the call is to the allowed recepient and amount is not more than allowed
        bytes calldata data;
        {
            uint256 offset = uint256(bytes32(_op.callData[4 + 64:4 + 96]));
            uint256 length = uint256(
                bytes32(_op.callData[4 + offset:4 + offset + 32])
            );
            //we expect data to be the `IERC20.transfer(address, uint256)` calldata
            data = _op.callData[4 + offset + 32:4 + offset + 32 + length];
        }
!        if (address(bytes20(data[16:36])) != recipient) {
            revert("ERC20SV Wrong Recipient");
        }
!        if (uint256(bytes32(data[36:68])) > maxAmount) {
            revert("ERC20SV Max Amount Exceeded");
        }
        return
            ECDSA.recover(
                ECDSA.toEthSignedMessageHash(_userOpHash),
                _sessionKeySignature
            ) == sessionKey;
    }

Impact

Data without length restrictions may lead to issues such as hash collisions. A hash collision may lead to the ability to arbitrarily forge user messages.

Recommendations

Use abi.decode to get the message length and add a maximum length check on op.callData.

Remediation

This issue has been acknowledged by Biconomy Labs, and a fix was implemented in commit 3bf128e9.

Zellic © 2024Back to top ↑