sherlock-audit / 2024-05-pooltogether-judging

13 stars 8 forks source link

aman - Griefing Attack: requests[requestId] Overflow Due to Large Request Volume and Short Block Time. #148

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

aman

medium

Griefing Attack: requests[requestId] Overflow Due to Large Request Volume and Short Block Time.

Summary

Each request from witnet is stored in requests mapping where the requestId type is uint32. it will overflow and return wrong block for each requester.

Vulnerability Detail

The witnet contract stores all the requests in following request mapping:

    mapping(uint32 requestId => uint256 lockBlock) public requests;

This request id got incremented for each new request despite the fact that if the request is made more the once in a same block.

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:     }
96: 

As mentioned in the ReadMe that these contract will also be deployed on Linea chain which block time is 2 sec. Keeping all these things in mind Following case would occur :

The user (Grief) will place the RNG request after first call in this way witnet will not charge any fee from users. The user can place multiple transaction As we know that transaction cost is very cheaper on Linea cost of approve call is $0.02 and block time 2 sec. Arbitrum : 2 sec time. Optimism : 2 sec time. let assume that if user place 2 request per block after a first call has been already made on the given block. The time it take to overflow can be calculated as Follows: The maximum value of a uint32 (unsigned 32-bit integer) is $2^{32} −1 = 4,294,967,2952 32−1 = 4,294,967,295$.

Now assume 3 increments per block:

$TotalBlock=4,294,967,295/ 3 ≈1,431,655,765$ Total time in seconds Linea: $ 1,431,655,765×2=2,863,311,530 seconds$ Now let convert these seconds in to days: $ 2,863,311,530/ 60 × 60 ×24 ≈ 33,140.87$ Total years required to overflow: $Total years = 33,140.87/365≈90.78 years

Keep in mind that these calculation are only in case of single user Grief attack. but the

Due to this overflow all the subsequent block number will be stored against 0 request id. The cost is not that High as the impact of this attacks that why I reported this issue.

Impact

The Request id for block will return wrong data to the draw manager which will impact the draw results.

Code Snippet

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

Tool used

Manual Review

Recommendation

change the type of requestId to uint256:

    mapping(uint256requestId => uint256 lockBlock) public requests;
sherlock-admin3 commented 5 months ago

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

infect3d commented:

overflow is desired here to reuse older requestIds

nevillehuang commented 5 months ago

Invalid, will not underflow since it is wrapped in an unchecked block, overflow is desirable and grief has no benefit