robertodf - Account abstraction wallets will not be able to claim rewards

[sherlock-admin4]

robertodf

High

Account abstraction wallets will not be able to claim rewards

Summary

While EOAs can have the same address across different networks due to the deterministic nature of the address derivation process, smart contract wallets will have different addresses on each network. In this case, the ticket manager is deployed on Avalanche, while the prize manager operates on Ethereum. For users interacting through a smart contract wallet, the address registered as the winner on Ethereum will differ from their actual address, preventing them from claiming their price.

Root Cause

Account abstraction wallets, even when associated with the same user, generate different addresses depending on the network. Since the ticket manager on Avalanche propagates the winner's address to the prize manager on Ethereum, this will lead to a missmatch. As a result, the winner may be unable to claim their prize on Ethereum because their smart contract wallet address differs from the one registered on Avalanche.

Internal pre-conditions

No specific internal preconditions are assumed.

External pre-conditions

Users must purchase tickets using a smart contract wallet.

Attack Path

1. User buys tickets using the smart wallet by calling WinnablesTicketManager::buyTickets: https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesTicketManager.sol#L182-L210
2. msg.sender is registered as the owner of the tickets in the ticket manager.
3. Once the raffle concludes and a winner is drawn, the winning ticket's associated address is propagated to the prize manager on Ethereum.
4. Winner claims the prize on Ethereum by calling WinnablesPrizeManager::claimPrize. The tx reverts as the user does not pass the following validation: https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesPrizeManager.sol#L120

Impact

Loss of users rewards.

PoC

The PoC is implemented using a forking test in Foundry for simplicity. Interactions involving the router and VRF coordinator are simplified and performed within the same test, without relying on off-chain components. To run the test, you'll need a Chainlink VRF subscription. The test is conducted on the Sepolia network. To execute it, load the RPC URL in a .env file and run:

forge test --mt testAccountAbstraction --fork-url $SEPOLIA_RPC_URL

See PoC

```javascript // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; import {Vm, Test} from "forge-std/Test.sol"; import {console2} from "forge-std/console2.sol"; import {WinnablesPrizeManager} from "../contracts/WinnablesPrizeManager.sol"; import {WinnablesTicket} from "../contracts/WinnablesTicket.sol"; import {WinnablesTicketManager} from "../contracts/WinnablesTicketManager.sol"; import "@chainlink/contracts-ccip/src/v0.8/ccip/libraries/Client.sol"; import "contracts/interfaces/IWinnablesPrizeManager.sol"; contract BaseTest is Test { address linkToken; address ccipRouter; address vrfCoordinator; uint64 subscriptionId = 7126; // your subscription ID bytes32 keyHash; WinnablesTicket tickets; WinnablesTicketManager ticketManager; WinnablesPrizeManager prizeManager; address admin; // protocol admin address signer; // signer: back-end API uint256 signerPk; address bobSCAVA; // Address of bob's SC account in Avalanche address bobSCETH; // Address of bob's SC account in Ethereum function setUp() external { (signer, signerPk) = makeAddrAndKey("signer"); bobSCAVA = makeAddr("bobSCAVA"); bobSCETH = makeAddr("bobSCETH"); // Sepolia Testnet configs linkToken = 0x779877A7B0D9E8603169DdbD7836e478b4624789; ccipRouter = 0x0BF3dE8c5D3e8A2B34D2BEeB17ABfCeBaf363A59; vrfCoordinator = 0x8103B0A8A00be2DDC778e6e7eaa21791Cd364625; keyHash = 0x474e34a077df58807dbe9c96d3c009b23b3c6d0cce433e59bbf5b34f823bc56c; // admin deploys contracts vm.startPrank(admin); prizeManager = new WinnablesPrizeManager(linkToken, ccipRouter); tickets = new WinnablesTicket(); ticketManager = new WinnablesTicketManager( linkToken, vrfCoordinator, subscriptionId, keyHash, address(tickets), ccipRouter ); vm.stopPrank(); } //////////////////////////// ///////// HELPERS ///////// ////////////////////////// // Helper function to create arbitrary message, since this is a fork test, no offchain components are being used // Specify in parameter data the parameters relevant for this test (e.g., addres of the winner) function exampleMessage( bytes memory data ) public returns (Client.Any2EVMMessage memory) { bytes32 messageId = 0x6f6c7e8b9e3d8b11b77fa05e7e6ed7839c23e0a1d1cf7c78d6b7512e7c8b51a1; // Example message ID uint64 sourceChainSelector = 1; // Example source chain selector bytes memory sender = abi.encode(address(this)); // Example sender address encoded // Example token amounts Client.EVMTokenAmount[] memory tokenAmounts = new Client.EVMTokenAmount[](1); tokenAmounts[0] = Client.EVMTokenAmount({ token: address(0), // Example token address amount: 0 // Example amount }); return Client.Any2EVMMessage({ messageId: messageId, sourceChainSelector: sourceChainSelector, sender: sender, data: data, destTokenAmounts: tokenAmounts }); } // Helper function simulating back-end server signed message used by Bob to buy tickets function bobBuyTicket(uint256 raffleId, uint16 ticketCount) public { bytes32 messageHash = keccak256( abi.encodePacked( bobSCAVA, uint256(0), raffleId, ticketCount, uint256(block.number), uint(0) ) ); // Create the prefixed hash that can be verified with ecrecover bytes32 ethSignedMessageHash = keccak256( abi.encodePacked("\x19Ethereum Signed Message:\n32", messageHash) ); // Sign the prefixed hash (uint8 v, bytes32 r, bytes32 s) = vm.sign( signerPk, ethSignedMessageHash ); // Combine r, s, and v into the signature bytes memory signature = abi.encodePacked(r, s, v); // Bob send tx signed by back-end server vm.prank(bobSCAVA); ticketManager.buyTickets( raffleId, ticketCount, block.number, signature ); } // Perform first steps until random number determining winner is picked. This is to avoid stack to deep error function startRaffle() public { // Fund contracts deal(address(prizeManager), 10 ether); deal(address(linkToken), address(prizeManager), 10 ether); vm.prank(admin); // lock price prizeManager.lockETH( address(ticketManager), uint64(14767482510784806043), // Fuji chain selector (forking from Sepolia) uint256(1), 1 ether ); // Simulate router call to indicate ticket manager that price was locked for raffleId 1 bytes memory data = abi.encode(uint8(1)); Client.Any2EVMMessage memory message = exampleMessage(data); vm.prank(admin); ticketManager.setCCIPCounterpart(address(this), 1, true); vm.prank(ccipRouter); ticketManager.ccipReceive(message); // Start the raffle in the ticket manager vm.prank(admin); uint256 raffleId = 1; uint64 startsAt = uint64(block.timestamp); uint64 endsAt = startsAt + 60; uint32 minTickets = 1; uint32 maxTickets = 3; uint32 maxHoldings = 2; ticketManager.createRaffle( raffleId, startsAt, endsAt, minTickets, maxTickets, maxHoldings ); // Prepare message that would be sent via back-end private key to buy tokens for Alice vm.prank(admin); ticketManager.setRole(signer, 1, true); //back-end API vm.prank(admin); tickets.setRole(address(ticketManager), 1, true); uint16 ticketCount = 2; // Buy two tickets for Bob bobBuyTicket(raffleId, ticketCount); // Raffle ends vm.warp(block.timestamp + 61); console2.log("Ticket Manager Address:", address(ticketManager)); // Add to consumer in Chainlink vm.recordLogs(); // Winner is selected for raffleId 1 ticketManager.drawWinner(1); Vm.Log[] memory entries = vm.getRecordedLogs(); // This is the genereated requestId in the fork test uint256 requestId = uint256(entries[1].topics[1]); uint256[] memory randomWords = new uint256[](1); // Simulate that random number is 2 randomWords[0] = 2; vm.prank(vrfCoordinator); // Simulate call from coordinator with random number determining winner ticketManager.rawFulfillRandomWords(requestId, randomWords); } ////////////////////////// ///////// TESTS ///////// //////////////////////// function testAccountAbstraction() public { startRaffle(); uint256 raffleId = 1; // Fund ticket manager to propagate winner to prize manager deal(address(linkToken), address(ticketManager), 10 ether); ticketManager.propagateRaffleWinner( address(prizeManager), uint64(14767482510784806043), uint256(raffleId) ); address winner = ticketManager.getWinner(uint256(1)); assertEq(winner, address(bobSCAVA)); bytes memory messageWinner = abi.encodePacked( uint8(IWinnablesPrizeManager.CCIPMessageType.WINNER_DRAWN), uint256(1), address(bobSCAVA) ); Client.Any2EVMMessage memory messageToSend = exampleMessage( messageWinner ); vm.prank(admin); prizeManager.setCCIPCounterpart(address(this), 1, true); vm.prank(ccipRouter); // Router tells prize manager address of the winner prizeManager.ccipReceive(messageToSend); vm.prank(bobSCETH); // Bob from his account on Ethereum now tries to claim his prize but since the addresses are different in each // network, he has lost the reward vm.expectRevert(abi.encodeWithSignature("UnauthorizedToClaim()")); prizeManager.claimPrize(raffleId); } } ```

Mitigation

Allow users to specifiy the address to which the prize should be assigned instead of using msg.sender in WinnablesTicketManager::buyTickets. For example:

See mitigation proposal

```javascript function buyTickets( uint256 raffleId, uint16 ticketCount, uint256 blockNumber, bytes calldata signature, address buyer ) external payable { if (ticketCount == 0) revert InvalidTicketCount(); _checkTicketPurchaseable(raffleId, ticketCount); _checkPurchaseSig(raffleId, ticketCount, blockNumber, signature, buyer); Raffle storage raffle = _raffles[raffleId]; uint256 participation = uint256(raffle.participations[buyer]); uint128 totalPaid = uint128(participation) + uint128(msg.value); uint32 totalPurchased = uint32(participation >> 128) + uint32(ticketCount); unchecked { raffle.participations[buyer] = bytes32( (participation & (type(uint256).max << 160)) | totalPaid | (uint256(totalPurchased) << 128) ); } unchecked { raffle.totalRaised += msg.value; _userNonces[buyer]++; _lockedETH += msg.value; } IWinnablesTicket(TICKETS_CONTRACT).mint(buyer, raffleId, ticketCount); IWinnablesTicket(TICKETS_CONTRACT).refreshMetadata(raffleId); } ```

Adjust _checkPurchaseSig accordingly.

[rdf5]

Escalate

Like many others have mentioned, I believe this should be classified as medium due to the growing adoption of this type of wallet. For a more detailed analysis, you can refer to the following dashboard on Dune, which tracks the evolution of ERC-4337 adoption in greater depth: https://dune.com/niftytable/account-abstraction

[sherlock-admin3]

Escalate

Like many others have mentioned, I believe this should be classified as medium due to the growing adoption of this type of wallet. For a more detailed analysis, you can refer to the following dashboard on Dune, which tracks the evolution of ERC-4337 adoption in greater depth: https://dune.com/niftytable/account-abstraction

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[jsmi0703]

122 and #260 is dup of this and also escalated.

[mystery0x]

This has been discussed with the HOJ agreeing that this commonly known issue non-rewarded and something the protocol chooses not to fix. Users should know ahead of time whether or not the receiving address is the same as the sending address. AA is that generally validation can include any logic. EIP4337 for instance allows that as well (any limitations on the validation step are purely done off-chain and not guaranteed by the protocol).

[WangSecurity]

I agree with the LJ on the part that the AA wallet owner should be aware of them not having the same addresses on different chains. But, of course, I would be glad to hear the arguments for the opposing side.

[rdf5]

Well, among other things, the purpose of AAs is precisely to simplify the use of this technology for less experienced users by abstracting away complexities, such as managing private keys or the need to hold the network’s native currency to pay gas fees. Given one of the target audiences of the smart wallets will be people seeking this simplified characteristics, I would expect from them to buy a ticket without noticing this. I understand your point of view, however, in my opinion, considering that one of the main goals in the space is to make Web3 more accessible and user-friendly to a broader audience, flawed mechanisms like this, feel like rowing in different directions.

[WangSecurity]

After additionally considering this issue, I agree it should be valid with medium severity. Out of all the duplicates, I've picked #122 to be the main report. As for more information about the decision, please refer to this comment under 122. Planning to accept the escalation and duplicate with #122

[WangSecurity]

UPD: track the discussion about this issue family under #122. If #122 is, in the end, invalid, planning to reject this escalation and leave the issue as it is. If #122 is valid, this escalation will be accepted, and this issue will be duplicated with #122.

[WangSecurity]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• rdf5: rejected