Cyfrin / 2023-08-sparkn

Other
10 stars 15 forks source link

Malicious/Compromised organiser can reclaw all funds, stealing work from supporters #506

Open codehawks-bot opened 10 months ago

codehawks-bot commented 10 months ago

Malicious/Compromised organiser can reclaw all funds, stealing work from supporters

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L116

Summary

The contest details state that 'If a contest is created and funded, there is no way to refund. All the funds belong to the persons who wants to help solve the problem, we call them "supporters".' (see More Context section). This is untrue, as the organizer is able to refund all of the contest funds.

Vulnerability Details

In Distributor#_distribute, there is no input validation on the winners array. A malicious or compromised organizer can, with little effort, simply pass an array of length one containing a wallet address that they control as the winners parameter, and [10000] as the percentages parameter in order to receive 100% of the funds initially deposited to the contract. Due to the design of the protocol, they would have 7 days after the contest ends (the value of the EXPIRATION_TIME constant in the ProxyFactory contract) to perform this action without the owner being able to prevent it.

Impact

Malicious/Compromised organizer can refund 100% of the contest funds, stealing work from sponsors.

Tools Used

Manual review

Recommendations

Use a two step procedure for distributing funds:

  1. The organizer submits an array of winners and percentages to the Proxy contract and they are cached using storage variables
  2. The owner of ProxyFactor (a trusted admin) checks the arrays to ensure the organizer is not distributing all of the money to themselves, and if satisfied, triggers the distribution of funds

This removes the risk of having to trust the organizer, and although it requires the trust of the admin, they were already a required trusted party and so the mitigation is beneficial overall. Also, this new system adds more truth to the statement from the contest details mentioned in the summary section of this report.

0xhahax0 commented 10 months ago

A malicious organizer is out of scope. The organizer is a trusted party, as stated in the known issues, by the sponsor and by a Cyfrin team-member:

Known-issues: We may build a reputation system in the futue to handle the issue of the system being maliciously used, e.g., sybil attack. Sponsor: sponsor sponsor2

Team: team1 team2 team3 team4

The SparkN team has very clearly communicated that are were a hybrid web-3 project: Unfortunately, the current condition of Web3's scalibility is not able to support total on-chain application. That is why we created a more versatile version of it using a hybrid solution of web2 and Web3. We will try to make it more and more decentralized after we onborad more users and the condition of Web3 improves.

This leaves very little attack-vectors left which I don't like but it is what it is. Therefor, this is not a valid issue.

EDIT: Adding, the communication from sponsors being in scope shouldn't come as something new. In previous contests, communication from sponsors has been taking as scope. Take a look at this medium finding in the Escrow contest.

I quote:

given the sponsor comment with regard to compatible tokens this doesn't appear to be addressed:

"For the moment assume the following:

WETH, USDC, LINK, DAI

But, the buyer and seller could do whatever they want - just we would recommend against that."
serial-coder commented 10 months ago

Escalate

This is an invalid issue. The organizer is considered trusted (confirmed by the developer).

romeroadrian commented 10 months ago

Escalate

Agree on invalid. This is clearly by design.

0xTenma commented 10 months ago

[Appeal] Malicious organizer is out of scope as per guidelines. I think this issue is Invalid.

4gkistrodon commented 10 months ago

Invalid, as the organizer is trusted. Furthermore, it is the intended and expected design that the organizers can choose how the funds are distributed.

wh1t3r05e commented 10 months ago

Agree on invalid