code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/AddressDriver.sol#L174 https://github.com/code-423n4/2023-01-drips/blob/main/src/NFTDriver.sol#L289

Vulnerability details

Impact

AddressDriver.sol and NFTDriver.sol uses OpenZeppelin’s safeApprove() which has been documented as (1) Deprecated because of approve-like race condition and (2) To be used only for initial setting of allowance (current allowance == 0) or resetting to 0 because it reverts otherwise. (see here).

Proof of Concept

2023-01-drips\src\AddressDriver.sol::174 => erc20.safeApprove(address(dripsHub), type(uint256).max); 2023-01-drips\src\NFTDriver.sol::289 => erc20.safeApprove(address(dripsHub), type(uint256).max);

Tools Used

Manual

Recommended Mitigation Steps

You should change it to increase/decrease Allowance as OpenZeppilin says.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

GalloDaSballo commented 1 year ago

Simply incorrect, the check is checking for 0 allowance, and if it's 0 it will approve max

code-423n4 / 2023-01-drips-findings