Users Are Forced To Give Maximum Allowance to Aave lendingPool

Summary

In a scenario were Aave's lendingPool is hacked or compromised, margin trading users will be at a high risk of getting their funds rekt

Vulnerability Detail

Here is the approval function:

    function _approveToken(address _address, address _tokenAddress, uint256 _tokenAmount) internal {
        if (IERC20(_tokenAddress).allowance(address(this), _address) < _tokenAmount) {
            IERC20(_tokenAddress).approve(_address, type(uint256).max); //@audit-issue max approval
        }
    }

Requiring users to grant such extensive allowance to Aave's lending pool is not ideal, as the unpredictable nature of DeFi hacks leaves their funds vulnerable.

Impact

In the event that the lending pool is compromised, an attacker could potentially exploit the unlimited approval granted by the _approveToken function to drain the entire balance of the user's margin trading contract

Code Snippet

https://github.com/sherlock-audit/2023-05-dodo/blob/930565dd875dac24609441423c7c76c2ae4719a8/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L392 https://github.com/sherlock-audit/2023-05-dodo/blob/930565dd875dac24609441423c7c76c2ae4719a8/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L363 https://github.com/sherlock-audit/2023-05-dodo/blob/930565dd875dac24609441423c7c76c2ae4719a8/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L374 https://github.com/sherlock-audit/2023-05-dodo/blob/930565dd875dac24609441423c7c76c2ae4719a8/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L385

Tool used

Manual Review and VS Code

Recommendation

I recommend that the approval function _approveToken be updated to approve the lending pool to spend only the necessary amount, rather than using type(uint256).max, which allows the lending pool to spend any amount. Additionally, if the user's current allowance for the lending pool is not enough to cover the necessary amount, the allowance should be increased to the required level. This will reduce the risk of funds being drained from the user's account in the event that the lending pool is hacked.

sherlock-audit / 2023-05-dodo-judging

Tendency - Users Are Forced To Give Maximum Allowance to Aave lendingPool #190