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

0 stars 2 forks source link

Caller contract doesn't have ability to invalidate signed message #110

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/Caller.sol#L90 https://github.com/code-423n4/2023-01-drips/blob/main/src/Caller.sol#L164-L183

Vulnerability details

Impact

Caller contract doesn't have ability to invalidate signed message.

Proof of Concept

Caller contract allows anyone to call allowed contract on behalf of account that allowed it. There is callSigned function, that user can use in order if he has signed message by sender. https://github.com/code-423n4/2023-01-drips/blob/main/src/Caller.sol#L164-L183

    function callSigned(
        address sender,
        address to,
        bytes memory data,
        uint256 deadline,
        bytes32 r,
        bytes32 sv
    ) public payable returns (bytes memory returnData) {
        // slither-disable-next-line timestamp
        require(block.timestamp <= deadline, "Execution deadline expired");
        uint256 currNonce = nonce[sender]++;
        bytes32 executeHash = keccak256(
            abi.encode(
                callSignedTypeHash, sender, to, keccak256(data), msg.value, currNonce, deadline
            )
        );
        address signer = ECDSA.recover(_hashTypedDataV4(executeHash), r, sv);
        require(signer == sender, "Invalid signature");
        return _call(sender, to, data, msg.value);
    }

This function is using nonce of sender in order to avoid replays. But there is no ability for sender to invalidate his signature, by increasing nounce for example. As result sender can't invalidate his signature in convenient way.

Tools Used

VsCode

Recommended Mitigation Steps

Add ability to increase nonce for sender.

GalloDaSballo commented 1 year ago

You can invalidate by signing a new one and making the nonce expire, also there's a deadline check

I think QA

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #261

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

L

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-c