sherlock-audit / 2023-05-dodo-judging

6 stars 2 forks source link

ak1 - MarginTrading.sol#L365 : Not checking the valid amount of transfer is not safe. #194

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

medium

MarginTrading.sol#L365 : Not checking the valid amount of transfer is not safe.

Summary

During _lendingPoolWithdraw does no check the valid amount of withdrawal.

Vulnerability Detail

During _lendingPoolWithdraw call, following will happen.

  1. approve x amount of tokens.
  2. withdraw the assets to contract address.

    null(address _asset, uint256 _amount, uint8 _flag) internal { _approveToken(address(lendingPool), _asset, _amount); lendingPool.withdraw(_asset, _amount, address(this)); emit LendingPoolWithdraw(_asset, _amount, _flag); }

Impact

When lending pool is unavailable or it has less amount of funds, withdraw can not happen. Though there is no actual withdrawal, contract still think the withdrawal happens and proceed for further transaction.

Code Snippet

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

Following places this function is called

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

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

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

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

Tool used

Manual Review

Recommendation

Check valid amount of withdraw

null(address _asset, uint256 _amount, uint8 _flag) internal {
    _approveToken(address(lendingPool), _asset, _amount);
    lendingPool.withdraw(_asset, _amount, address(this)); --------------- ???
    uint amount = lendingPool.withdraw(_asset, _amount, address(this));  ---------------------> +++++++
    require(amount > 0);
    emit LendingPoolWithdraw(_asset, _amount, _flag);
}