[M-01] Incorrect safeApprove usage

[code423n4]

Lines of code

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

Vulnerability details

[M-01] Incorrect safeApprove usage

Impact

The _transferFromCaller function for NFTDriver.sol uses ERC20 safeApprove() from OpenZeppelin's SafeERC20 library. However, the safeApprove function prevents changing an allowance between non-zero values to mitigate a possible front-running attack. It reverts if that is the case. Instead, the safeIncreaseAllowance and safeDecreaseAllowance functions should be used. Comment from the OZ library for this function: “// safeApprove should only be called when setting an initial allowance, // or when resetting it to zero. To increase and decrease it, use // 'safeIncreaseAllowance' and ‘safeDecreaseAllowance'"

Impact: If the existing allowance is non-zero (say, for e.g., previously the entire balance was not deposited due to vault balance limit resulting in the allowance being reduced but not made 0), then safeApprove() will revert causing the user’s token deposits to fail leading to denial-of-service. The condition predicate indicates that this scenario is possible.

Proof of Concept

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

Tools Used

Manual Analysis

Recommended Mitigation Steps

Use safeIncreaseAllowance() function instead of safeApprove().

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[GalloDaSballo]