code-423n4 / 2023-04-caviar-findings

9 stars 4 forks source link

CHANGEFEE IS NOT CORRECTLY SCALED IN FLASHLOAN() #961

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L623-L654 https://github.com/code-423n4/2023-04-caviar/blob/main/src/PrivatePool.sol#L750-L752

Vulnerability details

Impact

changeFee that has been scaled with 4 decimals of of basis points is being adopted by flashloan(). This could make the function behave in an unexpected manner than intended.

Proof of Concept

The fee is calculated as:

PrivatePool.sol#L632

        uint256 fee = flashFee(token, tokenId);

The returned changeFee is a very smaller integer. For example, if it is 1 USDC, that will mean 10000 USDC equivalent to USD 0.01 because USDC is associated with 6 decimals. It could have been worse if the baseToken has 18 decimals, which is as good as nothing.

PrivatePool.sol#L750-L752

    function flashFee(address, uint256) public view returns (uint256) {
        return changeFee;
    }

Additionally, it is going to get all flash loan easily pass and execute because of an easy bypass here (baseToken is ETH in this case):

PrivatePool.sol#L635

        if (baseToken == address(0) && msg.value < fee) revert InvalidEthAmount();

Recommended Mitigation Steps

It is recommended scaling changeFee like it has been done so in changeFeeQuote() before having it integrated with flashLoan().

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #864

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory