search

code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

`approveUnderlying` isn't safe #260

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0xsanson

Vulnerability details

Impact

In Basket.sol, approveUnderlying is used to approve tokens to be spent by the Auction. The current implementation uses a simple approve function, instead of the safer safeApprove. Also it's recommended to have an approve to zero first, since the allowance could be already positive and some tokens (e.g. USDT) wouldn't work in this case.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Basket.sol#L226

Tools Used

editor

Recommended Mitigation Steps

Consider writing

IERC20(tokens[i]).safeApprove(spender, 0);
IERC20(tokens[i]).safeApprove(spender, type(uint256).max);
GalloDaSballo commented 2 years ago

Duplicate of #35