search

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

0 stars 0 forks source link

`addLiquidity` Does Not Reset Approval If Not All Tokens Were Added To Liquidity Pool #228

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

leastwood

Vulnerability details

Impact

addLiquidity is called when users reinvest their tokens through bonding events. The RewardReinvestor first transfers Malt and rewards tokens before adding liquidity to the token pool. addLiquidity provides protections against slippage by a margin of 5%, and any dust token amounts are transferred back to the caller. In this instance, the caller is the RewardReinvestor contract which further distributes the dust token amounts to the protocol's treasury. However, the token approval for this outcome is not handled properly. Dust approval amounts can accrue over time, leading to large Uniswap approval amounts by the UniswapHandler contract.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L212-L214 https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L216-L218

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider resetting the approval amount if either maltUsed < maltBalance or rewardUsed < rewardBalance in addLiquidity.

GalloDaSballo commented 2 years ago

The UniV2Router will first calculate the amounts and then pull them from the msg.sender

This means that approvals may not be fully utilized, leaving traces of approvals here and there. This can cause issues with certain tokens (USDT comes to mind), and will also not trigger gas refunds.