Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuctionEschapeHatch.sol#exitEarly updates state of the auction wrongly #268

Open
code423n4 opened this issue Dec 1, 2021 · 1 comment
Open
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

0x0x0x

Vulnerability details

Vulnerability

AuctionEschapeHatch.sol#exitEarly takes as input amount to represent how much of the

When the user exits an auction with profit, to apply the profit penalty less maltQuantity is liquidated compared to how much malt token the liquidated amount corresponds to. The problem is auction.amendAccountParticipation() simply subtracts the malt quantity with penalty and full amount from users auction stats. This causes a major problem, since in _calculateMaltRequiredForExit those values are used for calculation by calculating maltQuantity as follow:

uint256 maltQuantity = userMaltPurchased.mul(amount).div(userCommitment);

The ratio of userMaltPurchased / userCommitment gets higher after each profit taking (since penalty is applied to substracted maltQuantity from userMaltPurchased), by doing so a user can earn more than it should. Since after each profit taking users commitment corresponds to proportionally more malt, the user can even reduce profit penalties by dividing exitEarly calls in several calls.

In other words, the ratio of userMaltPurchased / userCommitment gets higher after each profit taking and user can claim more malt with less commitment. Furthermore after all userMaltPurchased is claimed the user can have userCommitment left over, which can be used to claimArbitrage, when possible.

Mitigation Step

Make sure which values are used for what and update values which doesn't create problems like this. Rethink about how to track values of an auction correctly.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 1, 2021
code423n4 added a commit that referenced this issue Dec 1, 2021
@0xScotch 0xScotch added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 10, 2021
@GalloDaSballo
Copy link
Collaborator

The warden has identified an exploit that allows early withdrawers to gain more rewards than expected.
Anytime "points" and rewards need to be earned over time, it's ideal to accrue points in order to distribute them (see how Compound or AAVE tokens work)
Because the warden showed a flow in the accounting logic for the protocol, I agree with high severity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants