Open code423n4 opened 3 years ago
If the Auction contract has any ERC20 that is not checked against the require, those tokens can be taken away for free. The warden didn't directly mention this, but this can also apply to bounties. Any bounty specified in a token that is not protected can be taken without claiming the bounty (as the require won't check for it)
I think that the finding has appropriate severity, although the warden didn't directly mention the ability to steal bounties
Handle
gpersoon
Vulnerability details
Impact
Suppose some unrelated ERC20 tokens end up in the basket contract (via an airdrop, a user mistake etc)
Then anyone can do a bondForRebalance() and settleAuction() to scoop these tokens.
The function settleAuction() allows you to specify an outputToken, so also completely unrelated tokens. Thus you can retrieve additional tokens with settleAuction()
Proof of Concept
https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Auction.sol#L69 function settleAuction(.. address[] memory outputTokens, uint256[] memory outputWeights) public override { ... for (uint256 i = 0; i < outputTokens.length; i++) { IERC20(outputTokens[i]).safeTransferFrom(address(basket), msg.sender, outputWeights[i]); }
Tools Used
Recommended Mitigation Steps
Check outputTokens are part of the previous basket tokens (e.g. basket.tokens() )