code-423n4 / 2021-11-streaming-findings

0 stars 0 forks source link

Gas Optimization: Unnecessary flashloan checks #197

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

gzeon

Vulnerability details

Impact

The flashloan function in Locke.sol check balances of both deposit and reward tokens. While this is certainly the safest approach, it seems unnecessary to check for the balance of the token that was NOT flashloaned.

Proof of Concept

https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L696

Recommended Mitigation Steps

might consider rewrite to

function flashloan(address token, address to, uint112 amount, bytes memory data) public lock {
        require(token == depositToken || token == rewardToken, "erc");

        uint256 preTokenBalance = ERC20(token).balanceOf(address(this));
        ERC20(token).safeTransfer(to, amount);

        // the `to` contract should have a public function with the signature:
        // function lockeCall(address initiator, address token, uint256 amount, bytes memory data);
        LockeCallee(to).lockeCall(msg.sender, token, amount, data);

        uint256 postTokenBalance = ERC20(token).balanceOf(address(this));

        uint112 feeAmt = amount * 10 / 10000; // 10bps fee
        require(preTokenBalance + feeAmt <= postTokenBalance, "f1");

        if (token == depositToken) {
            depositTokenFlashloanFeeAmount += feeAmt;
        } else {
            rewardTokenFeeAmount += feeAmt;
        }

        emit Flashloaned(token, msg.sender, amount, feeAmt);
    }
0xean commented 2 years ago

dupe of #262