Submission Details

#16 Use `increaseAllowance()`/`decreaseAllowance()` instead of `approve()`/`safeApprove()`

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L198

Summary

Use increaseAllowance()/decreaseAllowance() instead of approve()/safeApprove()

Vulnerability Details

Changing an allowance with approve() brings the risk that someone may use both the old and the new allowance by unfortunate transaction ordering. Refer to ERC20 API: An Attack Vector on the Approve/TransferFrom Methods. It is recommended to use the increaseAllowance()/decreaseAllowance() to avoid this problem.

File: contracts/LiquidationPoolManager.sol

37:             eurosToken.approve(pool, _feesForPool);

76:                     ierc20.approve(pool, erc20balance);

Github: [37, 76]

File: contracts/SmartVaultV3.sol

198:         IERC20(_params.tokenIn).safeApprove(ISmartVaultManagerV3(manager).swapRouter2(), _params.amountIn);

Github: [198]

Impact

See #Vulnerability Details

Tools Used

Manual Review

Recommendations

Use increaseAllowance()/decreaseAllowance() instead of approve()/safeApprove()

Comments and Activity

Lead Judging Started

hrishibhat Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

allowance

informational/invalid

hrishibhat Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

allowance

informational/invalid