IceBear - Lender.sol flash() is a vulnerable function, can drain the asset

[sherlock-admin]

IceBear

medium

Lender.sol flash() is a vulnerable function, can drain the asset

Summary

Lender.sol flash() is a vulnerable function, can drain the asset

Vulnerability Detail

In the Lender.sol, the flash() function does not validate the 'to' address. It allows to pass an arbitrary bytes calldata data as one of the arguments. Later it calls receiver, that can be Lender contract itself, with a malicious data.

Impact

Malicious users can pass parameters like this:

flash（Lender, 1,abi.encodeWithSignature("approve(address,uint256)", attacker, amount));）

hacker will be approved and be able to transfer all tokens from the contract. P.S. The same issue was in Damn Vulnerably DeFi Challenges (Truster).

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Lender.sol#L295

Tool used

Manual Review

Recommendation

As per eip-3156. This is reference implementation in which initiator is authenticated.

    /// @dev ERC-3156 Flash loan callback
    function onFlashLoan(
        address initiator,
        address token,
        uint256 amount,
        uint256 fee,
        bytes calldata data
    ) external override returns(bytes32) {
        require(
            msg.sender == address(lender),
            "FlashBorrower: Untrusted lender"
        );
        require(
            initiator == address(this),
            "FlashBorrower: Untrusted loan initiator"
        );
        (Action action) = abi.decode(data, (Action));
        if (action == Action.NORMAL) {
            // do one thing
        } else if (action == Action.OTHER) {
            // do another
        }
        return keccak256("ERC3156FlashBorrower.onFlashLoan");
    }

Refer eip-3156 for reference.

[sherlock-admin2]

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because Lender contract doesn't have onFlashLoan function, so the example approve will simply revert

tsvetanovv commented:

I think the function works as it should

MohammedRizwan commented:

insufficient details