The `SafeApprove` Lib does not reset Approval to 0

[hats-bug-reporter[bot]]

Github username: @@amankakar Twitter username: -- Submission hash (on-chain): 0xad36662a5f0a2e1b7bdaefb4a2cb5313dfa0360e7f03e02e62f29b8f2bc3d18c Severity: medium

Description: Description\ Describe the context and the effect of the vulnerability.

Attack Scenario\ Describe how the vulnerability can be exploited. Description\ The SafeApprove lib is being used to grant approval and revoke approval form other contract like TwTap and YieldBox However The revoke of approval does not work as intended this could lead to potential issues. let have a look at usage of SafeApproval lib :

bigBang/BBCommon.sol:157
157:     function _depositAmountToYb(IERC20 token, address to, uint256 id, uint256 amount)
158:         internal
159:         returns (uint256 share)
160:     {
161:         address(token).safeApprove(address(yieldBox), amount);
162:         (, share) = yieldBox.depositAsset(id, address(this), to, amount, 0);
163:         address(token).safeApprove(address(yieldBox), 0);
164:     }

bigBang/BBLiquidation.sol:245
245:     function _extractLiquidationFees(uint256 returnedShare, uint256 borrowShare, uint256 callerReward)
246:         private
247:         returns (uint256 feeShare, uint256 callerShare)
248:     {
249:         uint256 extraShare = returnedShare > borrowShare ? returnedShare - borrowShare : 0;
250:         callerShare = (extraShare * callerReward) / FEE_PRECISION; //  y%  of profit goes to caller.
251:         feeShare = extraShare - callerShare; // rest of the profit goes to fee.
252: 
253:         //protocol fees should be kept in the contract as we do a yieldBox.depositAsset when we are extracting the fees using `refreshPenroseFees`
254:         if (callerShare > 0) {
255:             address(asset).safeApprove(address(yieldBox), type(uint256).max);
256:             yieldBox.depositAsset(assetId, address(this), msg.sender, 0, callerShare);
257:         }
258:         address(asset).safeApprove(address(yieldBox), 0);
259:     }

Now let's have a look at its implementation :

   function safeApprove(address token, address to, uint256 value) internal {
        require(token.code.length > 0, "SafeApprove: no contract");

        bool success;
        bytes memory data;
        (success, data) = token.call(abi.encodeCall(IToken.approve, (to, 0)));
        require(success && (data.length == 0 || abi.decode(data, (bool))), "SafeApprove: approve failed");

    @>    if (value > 0) {
            (success, data) = token.call(abi.encodeCall(IToken.approve, (to, value)));
            require(success && (data.length == 0 || abi.decode(data, (bool))), "SafeApprove: approve failed");
        }
    }

The safeApprove function first check if value>0 then it calls the approve function on token contract other wise it will do no thing. So in all the function call where the protocol passes 0 to safeApprove it will not reset/revoke the approvals from spender.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Matigation\ Implement forceApprove which make sure that approve function will be called disregards of 0. Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Files:

• tokenApproval.t.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmWaceVc3jTs5TDnAVkUSqPdFZZcDSNjCAKYStXgWgKWnp)

[amankakar]

Ignore this finding