`Ticket`: Owner of winning ticket can frontrun secondary sales

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L170-L177

Vulnerability details

The Wenwin docs note that tickets "can be traded on the secondary market before or after the draw," since they are standard ERC721 tokens.

After a ticket draw, the owner of a winning ticket may call Lottery#claimWinningTickets, which transfers lottery winnings to the ticket owner:

Lottery#claimWinningTickets:

    function claimWinningTickets(uint256[] calldata ticketIds) external override returns (uint256 claimedAmount) {
        uint256 totalTickets = ticketIds.length;
        for (uint256 i = 0; i < totalTickets; ++i) {
            claimedAmount += claimWinningTicket(ticketIds[i]);
        }
        rewardToken.safeTransfer(msg.sender, claimedAmount);
    }

A malicious winner can thus list their winning ticket on the secondary market and frontrun purchase transactions to collect both their winning reward and the secondary sale price of their ticket.

Scenario:

1. Mallory buys a Wenwin ticket.
2. Following the draw, the ticket is revealed as a Match 6 winner, worth 1500 DAI.
3. Mallory lists the winning ticket on a secondary marketplace like OpenSea for 1000 DAI.
4. Alice sees the ticket listed at a discount, and submits a transaction to purchase Mallory's listed ticket.
5. Mallory frontruns Alice's transaction and claims ticket winnings before the token is transferred to Alice.
6. Mallory receives both lottery winnings and secondary sale price. Alice cannot claim the lottery winnings from the ticket she purchased.

Recommendation: Consider a two step process for claiming awards: the end user can first submit their intent to claim, wait a fixed period of time, then finalize their claim.

Additionally, consider using Ticket token ERC721 metadata to clearly visually distinguish claimed and unclaimed tickets to mitigate secondary market scams. (For example, change the background color of claimed vs unclaimed ticket tokens). The current implementation has no token metadata for Ticket ERC721s, which will make it difficult to distinguish claimed/unclaimed and pre-draw/post-draw tickets trading on secondary marketplaces. Even a simple dynamic token SVG could mitigate many secondary market scams.

[c4-judge]

thereksfour marked the issue as primary issue

[c4-sponsor]

TutaRicky marked the issue as sponsor confirmed

[c4-judge]

thereksfour marked issue #366 as primary and marked this issue as a duplicate of 366

[c4-judge]

thereksfour marked the issue as satisfactory