sherlock-audit / 2024-07-sense-points-marketplace-judging

2 stars 0 forks source link

Pewbhai - Contracts that can receive ether but cannot send it may lock value permanently. #167

Closed sherlock-admin3 closed 2 weeks ago

sherlock-admin3 commented 2 weeks ago

Pewbhai

High

Contracts that can receive ether but cannot send it may lock value permanently.

Summary

In PointTokenValut.sol, the PointTokenVault contract can receive Ether due to the receive() function, but lacks functionality to send or withdraw it, potentially leading to permanently locked Ether.

Vulnerability Detail

The PointTokenVault contract includes a receive() function, allowing it to accept Ether. However, it lacks any functions to send or withdraw Ether, which can lead to a situation where any Ether sent to the contract is permanently locked. This issue arises because the contract is designed to handle ERC20 tokens and points-related logic, but it inadvertently allows Ether deposits without providing a mechanism for retrieval. This oversight can result in locked funds if Ether is mistakenly sent to the contract, as no functions are implemented to transfer Ether out.

falut

Impact

Locked Funds: The contract's receive() function allows it to accept Ether, but without any implemented function to transfer Ether out, any Ether sent to the contract becomes irretrievable.

User Error: Users interacting with the contract might mistakenly send Ether, assuming it supports Ether transactions or due to incorrect transaction inputs. Once Ether is sent, the absence of a withdrawal mechanism means these funds are locked indefinitely.

Example Scenario: The PointTokenVault contract is deployed on the Ethereum blockchain. A user, intending to interact with the contract's ERC20 functionality, mistakenly sends 1 Ether directly to the contract's address. The contract's receive() function is triggered, accepting the Ether. The Ether balance of the contract increases by 1 Ether. The user realizes the mistake and attempts to retrieve the Ether. The user finds no withdrawal function, leaving the Ether permanently locked in the contract. This scenario highlights the risk of having a receive() function without corresponding withdrawal functionality, leading to potential loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-07-sense-points-marketplace/blob/main/point-tokenization-vault/contracts/PointTokenVault.sol#L21

Tool used

Manual Review and SURYA by ConsenSys.

Recommendation

1) In the PointTokenVault contract, remove the receive() function entirely if Ether is not required for the contract's operations. This approach ensures that the contract cannot accept Ether, thereby eliminating the risk of accidental deposits.

2) Introduce a function, such as withdrawEther(), that allows the contract owner or an authorized role to transfer Ether out of the contract or else, always ensure that there is an ether exit for a contract that is designed to receive ether.