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

0 stars 0 forks source link

Set allowance only once when initializing a basket #78

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

neslinesli93

Vulnerability details

Impact

The approveUnderlying function calls safeApprove twice: once with 0, and finally with type(uint256).max. The first call wastes gas since the function is called only during basket initialization, thus the allowance attack does not apply here

Proof of Concept

https://github.com/code-423n4/2021-12-defiprotocol/blob/main/contracts/contracts/Basket.sol#L276

Recommended Mitigation Steps

Remove first call to safeApprove

0xleastwood commented 2 years ago

approveUnderlying() is called by setNewWeights(), so its necessary that it approves the zero amount beforehand as certain USDT-like tokens require this.