sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

slightscan - Flashloan received tokens amount isn't controlled #429

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

slightscan

medium

Flashloan received tokens amount isn't controlled

Summary

The Iron Bank protocol allows users to supply and borrow assets, and includes a flash loan feature. However, the flash loan logic does not control the end result of transferring tokens out and back in. This could potentially allow an attacker to steal the whole balance from Iron Bank.

As an example, this can be a kind of wide blacklisting mechanics introduction (i.e. allow this white list of accounts, freeze everyone else type of logic).

Vulnerability Detail

There is no control of the resulting balance, and any token that successfully performs safeTransfer, but for any reason withholds an update of token internal accounting, can successfully steal the lend flash loan amount from Iron Bank. This can be initiated by an attacker unrelated to token itself as griefing.

Impact

Some fungible tokens that qualify for Iron Bank (including not imposing any fee on transfers) may not return the whole amount back, but will report successful safeTransfer(). This can result in up to the flash loan amount of such ERC20 token being stolen. This can take place in a situation when a popular token was upgraded and the consequences of the internal logic change weren't fully understood by wide market initially and most lenders remained in Iron Bank, then someone calls a flash loan as a griefing attack that will result in the amount being stolen.

As the probability of such internal mechanics introduction is low, but the impact is up to full loss of user's funds, setting the severity to be medium.

Code Snippet

Flash loan functions do not employ any checks after ERC20 token was received back:

function _loan(IERC3156FlashBorrower receiver, address token, uint256 amount, bytes memory data, address msgSender)
    internal
{
    IronBankInterface(ironBank).borrow(address(this), address(receiver), token, amount);
    require(receiver.onFlashLoan(msgSender, token, amount, 0, data) == CALLBACK_SUCCESS, "callback failed"); // no fee
    IERC20(token).safeTransferFrom(address(receiver), address(this), amount);
    uint256 allowance = IERC20(token).allowance(address(this), ironBank);
    if (allowance < amount) {
        IERC20(token).safeApprove(ironBank, type(uint256).max);
    }
    IronBankInterface(ironBank).repay(address(this), address(this), token, amount);
}

https://github.com/sherlock-audit/2023-05-ironbank/blob/9ebf1702b2163b55479624794ab7999392367d2a/ib-v2/src/flashLoan/FlashLoan.sol/#L97

Tool used

Manual Review

Recommendation

Consider adding a balance control check to ensure that flash loan invariant remains: record contract balance before receiver_.onFlashLoan(...) callback and record it after IERC20(token).safeTransferFrom(address(receiver), address(this), amount); . Require that resulting token balance ends up being not less than initial.