sherlock-audit / 2023-06-real-wagmi-judging

3 stars 2 forks source link

dipp - Approval to swapTarget is not reset at the end of ```rebalanceAll``` in the multipool #155

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

dipp

high

Approval to swapTarget is not reset at the end of rebalanceAll in the multipool

Summary

Operators are able to give a swapTarget approval to a multipool's tokens indefinitely.

Vulnerability Detail

When an operator rebalances the multipool, the contract approves params.amountIn tokens to the swapTarget. After the swap, the function only checks that the params.amountIn value is more than the change in reserve for the token being sent to the swapTarget.

Multipool.sol#L868-L871

        uint256 amountIn = params.zeroForOne
            ? reserve0Before - reserve0
            : reserve1Before - reserve1;
        ErrLib.requirement(amountIn <= params.amountIn, ErrLib.ErrorCode.AMOUNT_IN_TO_BIG);

This allows the operator to approve swapTarget with a very large amountIn, much larger that actually used in the swap. The swapTarget will then still have the remaining approval to the multipool's tokens after the rebalance is finished.

Impact

If the swapTarget suffers an exploit that would allow it to spend any tokens approved to it, the multipool could lose all of its tokens.

Code Snippet

Multipool.sol#L845-L888

Tool used

Manual Review

Recommendation

Consider setting the approval of swapTarget to 0 at the end of the rebalanceAll function.

fann95 commented 1 year ago

fixed: https://github.com/RealWagmi/concentrator/blob/9036fe43b0de2694a0c5e18cfe821d40ce17a9b8/contracts/Multipool.sol#L932

0xffff11 commented 1 year ago

Fixed by changing the approval back to 0 after swapping