HardhatDeFi
15,000 USDC
View results
Submission Details
Severity: medium
Invalid

No Check for Non-zero Approval Before Token Transfer in `removeLiquidity`

Bug Description:
In the AaveDIVAWrapper.sol contract, specifically within the _removeLiquidity function, there is an implicit assumption that the user has approved the contract to transfer their short and long tokens without explicitly checking.

// Transfer short and long tokens from user to this contract. Requires prior user approval on both tokens.
// No need to use `safeTransferFrom` here as short and long tokens in DIVA Protocol are standard ERC20 tokens
// using OpenZeppelin's ERC20 implementation.
_shortTokenContract.transferFrom(msg.sender /** from */, address(this) /** to */, _positionTokenAmountToRemove);
_longTokenContract.transferFrom(msg.sender /** from */, address(this) /** to */, _positionTokenAmountToRemove);

This code assumes that the user has already given approval for the contract to use their tokens. However, if the user has not given this approval or if the approval has been revoked, these transferFrom calls will revert, leading to transaction failure. There's no check to verify the approval amount before attempting the transfer.

Impact:
The primary impact is transaction reversion due to inadequate permission, which not only frustrates users but also affects the reliability and user experience of the contract's functionality.

Mitigation:
Add checks for the allowance of both short and long tokens before attempting to transfer.

Updates

Lead Judging Commences

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.