`_transferFromCaller` is not compatible with `USDT` and similar tokens

[code423n4]

Lines of code

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

Vulnerability details

_transferFromCaller is not compatible with USDT and similar tokens

Summary

Setting directly type(uint256).max won't work for USDT(Tether). This is done at both _transferFromCaller:

• NFTDriver.sol#L289

• AddressDriver.sol#L174

Description

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

Also approve() will fail for certain token implementations that do not return a boolean value. Hence it is recommend to use increaseAllowance() and decreaseAllowance()

Vulnerability Detail

approve reverts for tokens like USDT if first not approved to 0 Use of approve over is not suggested for many issues.

Setting directly type(uint256).max won't work for USDT(Tether)

Impact

Reverting in tokens like USDT(Tether), therefore not compatible at _transferFromCaller

Code Snippet

• Some ERC20 like USDT fail if not set first to 0

• NFTDriver.sol#L289

• AddressDriver.sol#L174

Tool used

Manual Review

Recommendation

• Add approve(0) before approve(uint)
• Consider using increaseAllowance and decreaseAllowance instead of approve

[GalloDaSballo]

[GalloDaSballo]

If it's 0 then we approve to max

Else we don't do anything

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid