code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

Unlimited settlements may lead to losses for creators #404

Closed c4-bot-6 closed 9 months ago

c4-bot-6 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/AuctionHouse.sol#L152-L155 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/AuctionHouse.sol#L161-L163 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/AuctionHouse.sol#L336-L414

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

Settling the last bid will handle the logic:

  1. transfer the ntf to the successful bidder or burn the ntf if no one bid.
  2. Distribute the acquired earnings
    1. Part of the earnings are sent to the owner
    2. Split the creator's earnings into 2 parts. 1.Part of the income will be sent to the creator in eth. 2.The other part is to help the creators to buy ERC20TokenEmitter.

The problem occurs in the part of buying ERC20TokenEmitter for the creators. At this point, a bad user may run a settlement at a high price, letting the creator buy the ERC20TokenEmitter at a high price. We know that the price of ERC20TokenEmitter is set based on the VRGDA curve, and when there are too many purchasing users, the price will increase exponentially, and settlement at this time is very unfavorable to creators.

This problem is not the same as slippage protection, there is slippage protection, bad users can still run settlement at ERC20TokenEmitter high price.

Tools Used

Manual Review

Recommended Mitigation Steps

Add access control to the settlementCurrentAndCreateNewAuction and settlementAuction functions to avoid being controlled by by bad users.

Assessed type

Access Control

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as primary issue

raymondfam commented 10 months ago

Intended design of VRGDA.

c4-judge commented 9 months ago

MarioPoneder marked the issue as unsatisfactory: Invalid

c4-judge commented 9 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

MarioPoneder marked the issue as grade-c

rocketman-21 commented 9 months ago

i don't see the attack motivation here, and the tokens the attacker would have to buy are non transferable so it is very costly to do this imho

wangxx2026 commented 9 months ago

Motivation can be an example that you use as a reference. For example, malicious competition, deliberately lowering the voting power of people you hate. And as a user who needs to use his voting rights, launching an attack, he gains the rights he wants, lowering the rights of his victims

MarioPoneder commented 9 months ago

Thank you for your comment!

This report contains insufficient proof and description to further follow-up on this attack vector which is moreover infeasible due to the involved costs at little gain for the attacker.