sherlock-audit / 2023-02-bond-judging

2 stars 0 forks source link

ss3434 - Must approve by zero first #12

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ss3434

medium

Must approve by zero first

Summary

Some ERC20 tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. For example Tether (USDT)'s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals.

Vulnerability Detail

Some tokens will revert when updating allowance. They must first be approved by zero and then the actual allowance must be approve

Impact

The protocol will impossible to use with certain tokens.

Code Snippet

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondBatchAuctionV1.sol#L151

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondBatchAuctionV1.sol#L167-L170

Tool used

Manual Review

Recommendation

It is recommended to set the allowance to zero before increasing the allowance.

Change this:

batchAuctionParams_.payoutTokenParams.underlying.approve(address(teller()), amount);

To this :

+     batchAuctionParams_.payoutTokenParams.underlying.approve(address(teller()), 0);
      batchAuctionParams_.payoutTokenParams.underlying.approve(address(teller()), amount);

and

     payoutToken.approve(
                address(gnosisAuction()),
                (uint256(batchAuctionParams_.auctionAmount) *
                    (gnosisAuction().feeNumerator() + feeDecimals)) / feeDecimals
            );

To this:


+    payoutToken.approve(address(gnosisAuction()), 0);
     payoutToken.approve(
                address(gnosisAuction()),
                (uint256(batchAuctionParams_.auctionAmount) *
                    (gnosisAuction().feeNumerator() + feeDecimals)) / feeDecimals
            );

Duplicate of #5

Oighty commented 1 year ago

This is a duplicate of #4 and #5 which are disputed.