code423n4 commented 2 years ago

Handle

defsec

Vulnerability details

Impact

Some tokens (like USDT L199) 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.

IERC20(token).safeApprove(address(operator), 0);
IERC20(token).safeApprove(address(operator), amount);

When trying to re-approve an already approved token, all transactions revert and the protocol cannot be used.

Proof of Concept

(https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/a5c71b72222d84b6414ca0339ed1761dc79fe56e/contracts/SettToRenIbbtcZap.sol) (https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/6f700995129182fec81b772f97abab9977b46026/contracts/IbbtcVaultZap.sol) (https://github.com/Badger-Finance/ibbtc/blob/d8b95e8d145eb196ba20033267a9ba43a17be02c/contracts/Zap.sol)

Tools Used

None

Recommended Mitigation Steps

Approve with a zero amount first before setting the actual amount.

GalloDaSballo commented 2 years ago

Disagree with the finding, we are setting from 0 (as we just deployed) to max uint and we never set approvals again. No need to set the approval to 0 if it already is

0xleastwood commented 2 years ago

agree with sponsor, marking invalid

code-423n4 / 2021-11-badgerzaps-findings