search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

Must approve 0 first #13

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/CvxCrvRewardsLocker.sol#L53

Vulnerability details

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

    approve without approving 0 first CvxCrvRewardsLocker.sol, 52,         IERC20(CRV).safeApprove(CRV_DEPOSITOR, type(uint256).max);
    approve without approving 0 first Swapper3Crv.sol, 43,         IERC20(USDC).safeApprove(SUSHISWAP, type(uint256).max);
    approve without approving 0 first Swapper3Crv.sol, 46,         IERC20(DAI).safeApprove(UNISWAP, type(uint256).max);
    approve without approving 0 first BkdTriHopCvx.sol, 70,         IERC20(underlying_).safeApprove(curveHopPool_, type(uint256).max);
    approve without approving 0 first CvxCrvRewardsLocker.sol, 58,         IERC20(CRV).safeApprove(CVX_CRV_CRV_CURVE_POOL, type(uint256).max);
chase-manning commented 2 years ago

This approval is being called in the constructor. It is guaranteed that this has not been approved already because the contract doesn't exist yet.

gzeoneth commented 2 years ago

Agree with sponsor, not an issue for these 5 specific cases.