Attacker can create a AuctionCrowdfund and rug any contributions made by other users #198
Labels
bug
Something isn't working
disagree with severity
Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments)
old-submission-method
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
sponsor acknowledged
Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Lines of code
https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/AuctionCrowdfund.sol#L178-L181
https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/globals/LibGlobals.sol#L5
Vulnerability details
Description
Users can set up AuctionCrowdfunds using createAuctionCrowdfund(). They are free to pass any market wrapper, which is checked in the initialize function:
After other users contribute() to the Crowdfund, anyone can call the bid() function. It wraps the MarketWrapper's bid() call:
Unfortunately, this pattern is vulnerable because a delegatecall to user supplied contract leads to undefined behavior. In this case, user can craft an attack contract passed as market wrapper. After publishing the auction and receiving contributions, he can call bid() to redirect execution to his contract. The contract can simply selfdestruct() to kill the Proxy and send the money to the attacker's wallet.
Impact
Any contributions made to a Crowdfund with a not-whitelisted market wrapper can be stolen by the fund creator.
Proof of Concept
Tools Used
Manual audit
Recommended Mitigation Steps
If PartyDAO intends to use delegatecall design choice, it must manage a whitelist of approved MarketWrapper contracts ( right now there are 4). These should be stored in the Globals contract and checked during Crowdfund initialize().
The text was updated successfully, but these errors were encountered: