code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

`AuctionDemo.sol` has no way to rescue funds. #2004

Closed c4-submissions closed 6 months ago

c4-submissions commented 7 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L0-L151

Vulnerability details

Description

AuctionDemo.sol has no function to rescue funds. So, any non-bidder ether sent to the contract is forever lost. Also, as my other reports states, the contract is vulnerable to gas-griefing and out-of-gas error, which are attacks that locks funds and can be softened if a emergency withdraw function exists.

Proof of Concept

  1. The contract suffered a denial of service, like out-of-gas and gas-griefing attacks. Or some ether was sent by mistake to it.
  2. The contract can't rescue any funds from the mistake or the attacks, because there is no emergency withdraw.

Impact

Protocol can't rescue funds from attacks or mistakes.

  1. Likelihood: Medium. In a scenario that the contract isn't vulnerable to the reported gas-griefing and out-of-gas exploits, still can happen another attacks or wrong sent ether, so it's better to be prevented.
  2. Impact: Medium. The cases where emergency withdraw is needed but don't exist represents loss of funds.
    • Risk: Medium (Medium Likelihood + Medium Impact)

Tools Used

Manual Review

Recommended Mitigation Steps

Add an emergencyWithdraw function.

Assessed type

ETH-Transfer

c4-pre-sort commented 7 months ago

141345 marked the issue as sufficient quality report

c4-sponsor commented 7 months ago

a2rocket (sponsor) confirmed

alex-ppg commented 7 months ago

The Warden specifies that the Sponsor should introduce some form of emergency withdrawal mechanism in their Auction system.

While a correct recommendation, the warden relies on a separate exhibit to justify it which I deem invalid. Additionally, funds cannot be accidentally sent to the contract as it has no receive mechanism or other payable method and can only accept funds "deliberately" (i.e. recipient of coin base rewards or selfdestruct recipient).

As such, I will downgrade to QA.

c4-judge commented 7 months ago

alex-ppg changed the severity to QA (Quality Assurance)