sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

0xMaroutis - Malicious attacker can drain funds via `flashLoan` function #173

Closed sherlock-admin closed 10 months ago

sherlock-admin commented 10 months ago

0xMaroutis

high

Malicious attacker can drain funds via flashLoan function

Summary

An attacker could exploit the function to obtain a flash loan without doing any previous transfer by updating the reserves during callback.

Vulnerability Detail

The vulnerability arises from the ability to manipulate reserves through the sync function within the flashLoan callback. An attacker, by creating a contract implementing IDODOCallee with a callback function DSPFlashLoanCall that calls .sync() function, can update the reserves during the process, which allows him to bypass any balance checks.

By executing the sync function the following checks will pass :

require(
                (uint256(_BASE_RESERVE_) - baseBalance) <= receiveBaseAmount,
                "FLASH_LOAN_FAILED"
            );
require(
                (uint256(_QUOTE_RESERVE_) - quoteBalance) <= receiveQuoteAmount,
                "FLASH_LOAN_FAILED"
            );

Because when the reserve sync up both :

uint256(_BASE_RESERVE_) - baseBalance = 0

&&

uint256(_QUOTE_RESERVE_) - quoteBalance = 0

The checks will pass which allows the attacker to take/swaps tokens without any transfers.

Impact

Attacker can drain the funds of depositors.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPVault.sol#L118-L137 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L131-L132

Tool used

Manual Review Foundry

Recommendation

Consider removing the callback DSPFlashLoanCall or adding it only after the checks/requires of balances.

Duplicate of #42