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

theft of system profit #56

Open
code423n4 opened this issue Nov 27, 2021 · 3 comments
Open

theft of system profit #56

code423n4 opened this issue Nov 27, 2021 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working needs additional sponsor input determined to need input from sponsor during sponsor review validation checks

Comments

@code423n4
Copy link
Contributor

Handle

danb

Vulnerability details

Impact

System profit comes from the stabilize function when the price of malt is above 1. Therefore it should not be allowed for anyone to take a part of the system profit when the price is above one. Right now, anyone can take a part of the investors' profits, even if they don't own any malt at all.

Proof of Concept

suppose that the price went up to 1.2 dai; the investors should get the reward for this rise. instead, anyone can take a part of the reward in the following steps:
The price of malt is now 1.2 dai.
Take a flash loan of a large amount of malt.
Sell the malt for dai.
Now the price went down to 1.1 dai because of the enormous swap.
Call stabilize in order to lower the price to 1 dai.
Buy malt to repay the flash loan at a lower cost with the bought dai.
Repay the flash loan, and take the profit.

It is a sandwich attack because they sold malt for a high price, then they called stabilize to lower the value, then repurchase it for a low price.

The user made a profit at the expense of the investors.

If a user already has malt, it’s even easier:
just sell all malt at a high cost.

Tools Used

manual review.

Recommended Mitigation Steps

in _beforeTokenTransfer, if the price is above 1$ and the receiver is the AMM pool, stabilize it.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 27, 2021
code423n4 added a commit that referenced this issue Nov 27, 2021
@loudoguno loudoguno added the needs additional sponsor input determined to need input from sponsor during sponsor review validation checks label Dec 17, 2021
@GalloDaSballo
Copy link
Collaborator

@0xScotch The warden says that if the price is above the peg price, the system should stabilize it
From reading the PoolTransferVerification.verifyTransfer, the system wants you to trade when Malt is above peg (minus tolerance)

Would you say this finding is invalid?

@0xScotch
Copy link
Collaborator

@0xScotch The warden says that if the price is above the peg price, the system should stabilize it From reading the PoolTransferVerification.verifyTransfer, the system wants you to trade when Malt is above peg (minus tolerance)

Would you say this finding is invalid?

I think the issue being pointed out is that the stabilization above peg can be sandwiched and some profit that would go to LPs can be extracted. I think this is an issue but its not absolutely critical.

We have discussed internally how to deal with this and the suggestion of calling stabilize would require some additional logic in the transfer verification as it would create a circular execution path.

  1. User tries to sell at 1.2
  2. Transfer verifier triggers stabilize
  3. StabilizerNode tries to sell Malt ahead of initial user
  4. Transfer verifier runs again and will call stabilize again unless we modify it to deal with this case.

I don't think that is an issue but we will consider how to best move ahead with it.

@GalloDaSballo
Copy link
Collaborator

Let's dissect the warden's finding:

The price of malt is now 1.2 dai.
Take a flash loan of a large amount of malt.
Sell the malt for dai.
Now the price went down to 1.1 dai because of the enormous swap.
Call stabilize in order to lower the price to 1 dai.
Buy malt to repay the flash loan at a lower cost with the bought dai.
Repay the flash loan, and take the profit.

-> Flashloan Malt (assumes liquidity to do so, but let's allow that)
-> Sell malt for dai -> price goes lower
-> Call stabilize -> system reduces price even further
-> Buy back the malt
-> repay the loan

I feel like this is something that can't really be avoided as due to the permissionless nature of liquidity pools, limiting the ability to sell or buy to a path will be sidestepped by having other pools being deployed to avoid those checks.

That said, the warden has identified a way to leak value from the price control system.

To be precise the value is being extracted against traders, as to get Malt to 1.2 you'd need a trader to be so aggressive in their purchase to push the price that high.
The warden showed how any arber / frontrunner can then "steal" the potential profits of the malt system users by frontrunning them.

Due to the profit extraction nature of the finding, I believe Medium Severity to be correct

@GalloDaSballo GalloDaSballo added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jan 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working needs additional sponsor input determined to need input from sponsor during sponsor review validation checks
Projects
None yet
Development

No branches or pull requests

4 participants