code-423n4 / 2023-05-caviar-mitigation-contest-findings

0 stars 0 forks source link

Mitigation Confirmed for H-02 #25

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Mitigation of H-02: Issue mitigated

Issue

H-02: PrivatePool owner can steal all ERC20 and NFT from user via arbitrary execution

Mitigation

https://github.com/outdoteth/caviar-private-pools/pull/2

Assessment of Mitigation

Issue mitigated

Comments

This mitigation adds the following InvalidTarget error in the PrivatePool contract and updates the PrivatePool.execute function to execute if (target == address(baseToken) || target == address(nft)) revert InvalidTarget().

Because calling the PrivatePool.execute function when target is baseToken or nft now reverts, the private pool owner cannot use this function to transfer tokens of baseToken or nft for which users have approved the PrivatePool contract as an operator. Thus, the corresponding issue is mitigated. This change also mitigates M-15: Pool tokens can be stolen via PrivatePool.flashLoan function from previous owner since the private pool owner can no longer call the PrivatePool.execute function to approve other contract or EOA as the operator for the private pool's baseToken and nft tokens before becoming the previous owner of the same private pool.

https://github.com/outdoteth/caviar-private-pools/pull/2/files#diff-6beea546a01e795e1365ca8a74b2bd952631f6cd228a5a869bfed82bf1f46a0fR80

error InvalidTarget();

https://github.com/outdoteth/caviar-private-pools/pull/2/files#diff-6beea546a01e795e1365ca8a74b2bd952631f6cd228a5a869bfed82bf1f46a0fR460-R479

    function execute(address target, bytes memory data) public payable onlyOwner returns (bytes memory) {
        if (target == address(baseToken) || target == address(nft)) revert InvalidTarget();

        // call the target with the value and data
        (bool success, bytes memory returnData) = target.call{value: msg.value}(data);

        // if the call succeeded return the return data
        if (success) return returnData;
        // if we got an error bubble up the error message
        if (returnData.length > 0) {
            // solhint-disable-next-line no-inline-assembly
            assembly {
                let returnData_size := mload(returnData)
                revert(add(32, returnData), returnData_size)
            }
        } else {
            revert();
        }
    }
c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory