search

sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

0x73696d616f - `Requestor` uses `to.transfer()` to withdraw the balance of the creator, but the creator may not be able to receive it #98

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 3 months ago

0x73696d616f

medium

Requestor uses to.transfer() to withdraw the balance of the creator, but the creator may not be able to receive it

Summary

address.transfer() only forwards 2300 gas, which can easily make gas sent revert if the target is a smart contract wallet. Thus, users participating in the draws (which is permissionless, and intends to allow anyone to participate, as in the docs), may not be able to withdraw their funds after funding rng requests.

Vulnerability Detail

The auction in DrawManager is started by calling RngWitnet::startDraw(), which requests a random number from Requestor::randomize() and then DrawManager::startDraw().

Users starting the auction can choose to send funds to the Requestor directly or as msg.value in the RngWitnet::startDraw() call. Either way, funds are expected to be leftover in the Requestor contract, as it is not possible to predict the exact gas price and corresponding fee (the fee is based on the gas price) that will be used when the RngWitnet::startDraw() is called. Also, requests that have already been requested for the same block number have their msg.value refunded.

Lastly, the funds are always sent to the msg.sender in requestors[msg.sender], corresponding to the Requestor, and RngWitnet is the creator, which has no way of specifying a different address to send the funds to.

Thus, as PoolTogether incentives the participation of anyone, some users using smart wallets may request randomness, paying the fee and leaving the excess in the Requestor contract, and then when they try to withdraw, it fails.

Impact

Stuck tokens due to not being able to withdraw funds from the Requestor.

Code Snippet

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

Tool used

Manual Review

Vscode

Recommendation

Use address.call{value: address(this).balance)}("") to send the funds.

Duplicate of #80

sherlock-admin2 commented 2 months ago

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

infect3d commented:

invalid__ only affect bots that deliberately chose to set code inside the fallback function