search

code-423n4 / 2022-04-dualityfocus-findings

1 stars 0 forks source link

FlashLoan can miss borrowed funds repayment #44

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-dualityfocus/blob/main/contracts/vault_and_oracles/FlashLoan.sol#L57

Vulnerability details

Impact

FlashLoan doesn't control for the success of funds return transfer call and can end up losing all the amount loaned whenever assets[0] ERC20 do not revert on operation failure.

The assets[0] can be an arbitrary ERC20, the implementation currently doesn't white list a subset of tokens for flash loans.

Placing severity to high here as loan payback is a crucial flash loan step, and if LP_VAULT do not controlled to always return lent funds back, the whole balance of flash loan provider can be stolen via such a loophole. I.e. using non-reverting assets[0] and creating a situation when there is not enough funds (simply spending them anyhow), an attacker can stole the whole assets[0] balance of the flash loan provider. As the system's main focus is LP NFT collateralization (i.e. the main focus is elsewhere), the probability of not tight enough control so such an ERC20 be added to operations is not negligible.

Proof of Concept

FlashLoan's executeOperation use transfer without result check for an arbitrary assets[0]:

https://github.com/code-423n4/2022-04-dualityfocus/blob/main/contracts/vault_and_oracles/FlashLoan.sol#L57

assets[0] can be ERC20 that doesn't revert on transfer failure:

https://github.com/d-xo/weird-erc20#no-revert-on-failure

Recommended Mitigation Steps

Consider adding transfer operation result control either directly, or by using safeTransferFrom

0xdramaone commented 2 years ago

We do not see any risk as described.

FlashLoan.sol is an implementation of the Aave flashloan receiver, not our own flashloan functionality. Flashloan receiver does not hold funds except when taking a temporary flashloan within a transaction. The flashloan receiver will revert if the amount lent + premium are not payed back. The assets that can be flashloaned are limited to those supported by Aave, and if the flashLoan.sol contract does not get enough funds back it will simply revert.

jack-the-pug commented 2 years ago

This is not a valid issue.

FlashLoan.sol is not the provider of the flashloan, but the receiver. Not repaying enough will revert the flash loan transaction.

And assets[0] can not be an arbitrary ERC20, it must be one of the tokens supported by the lending pool.