sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

aman - Reorgs attack issue while deploying using create , will result in fund lost for Requester contract #77

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 3 months ago

aman

high

Reorgs attack issue while deploying using create , will result in fund lost for Requester contract

Summary

The Requester contract is used to request RNG from witnet protocol, However The Requester contract is deployed using create op-code which is vulnerable to re orgs attacks and also hold the fee amount left.

Vulnerability Detail

When user request for new RNG the Protocol checks that if the user has requester deployed if not then the protocol deployed requester contract using create op-code after this deployment the newly created requester contract is used to request new RNG from witnet and witnet return the left amount to the newly deployed request contract has it sends the request to witnet contract.

2024-05-pooltogether/pt-v5-rng-witnet/src/RngWitnet.sol:51
51:     function getRequestor(address user) public returns (Requestor) {
52:         Requestor requestor = requestors[user];
53:         if (address(requestor) == address(0)) {
54:             requestor = new Requestor(); 
55:             requestors[user] = requestor;
56:         }
57:         return requestor;
58:     }

The request Function call :

/home/aman/Desktop/audits/2024-05-pooltogether/pt-v5-rng-witnet/src/RngWitnet.sol:85
85:     function requestRandomNumber(uint256 rngPaymentAmount) public payable returns (uint32 requestId, uint256 lockBlock, uint256 cost) {
86:         Requestor requestor = getRequestor(msg.sender);
87:         unchecked {
88:             requestId = ++lastRequestId; // @audit-issue : will overflow
89:             lockBlock = block.number;
90:         }
91:         requests[requestId] = lockBlock;
92:         cost = requestor.randomize{value: msg.value}(rngPaymentAmount, witnetRandomness);
93: 
94:         emit RandomNumberRequested(requestId, msg.sender, rngPaymentAmount, cost);
95:     }

92: it will submit request to witnet via newly created requester. The amount left will be sent back to new Requester. Note that for new Requester the deployment and submission of request happen in same block. Re-orgs is a known issue in Ethereum and other EVM compatible protocols and one depth re-orgs is pretty common. Alice Tries to request new RNG for the first time.

  1. The Alice calls requestRandomNumber function to generate new RNG.
  2. The contract deploy new Request instance for Alice.
  3. The new Request contract submit the transaction to witnet.
  4. The Witnet add request to its state and return the remaining Funds to new Request contract.
  5. Execution completed.
  6. Block re-orgs, Alice will not be able to withdraw her funds

Impact

The user will lost the funds locked in Requester contract.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-rng-witnet/src/Requestor.sol#L36-L40 https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-rng-witnet/src/RngWitnet.sol#L85-L95

Tool used

Manual Review

Recommendation

deployed the contract with create2 op-code and use the user address as a salt. it is already done with the PrizeVault contract.

sherlock-admin2 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

re-org are invalid by sherlock rules unless specified by sponsor as valid

nevillehuang commented 2 months ago

Invalid per sherlock rules:

Chain re-org and network liveness related issues are not considered valid.