qpzm - `MarginTrading._lendingPoolWithdraw` approves unnecessarily.

[sherlock-admin]

qpzm

medium

MarginTrading._lendingPoolWithdraw approves unnecessarily.

Summary

MarginTrading._lendingPoolWithdraw approves unnecessarily.

Vulnerability Detail

Aave v2 LendingPool.withdraw burns the AToken, without transferFrom. Thus, approve is not necessary before withdraw.

When I change this line to _approveToken(address(lendingPool), _asset, 0); the tests all pass.
https://github.com/sherlock-audit/2023-05-dodo/blob/main/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L364

Impact

Unnecessary approval of ERC20 token to aave lending pool.

Code Snippet

https://github.com/sherlock-audit/2023-05-dodo/blob/main/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L363-L364

https://github.com/aave/protocol-v2/blob/master/contracts/protocol/lendingpool/LendingPool.sol#L179

Tool used

Manual Review

Recommendation

Delete the line 364 _approveToken(address(lendingPool), _asset, _amount);