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

User can bypass Recovery Mode via UniswapHandler to buy Malt #325

Open
code423n4 opened this issue Dec 1, 2021 · 2 comments
Open

User can bypass Recovery Mode via UniswapHandler to buy Malt #325

code423n4 opened this issue Dec 1, 2021 · 2 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 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

gzeon

Vulnerability details

Impact

One of the innovative feature of Malt is to block buying while under peg. The buy block can be bypassed by swapping to the whitelisted UniswapHandler, and then extract the token by abusing the add and remove liquidity function. This is considered a high severity issue because it undermine to protocol's ability to generate profit by the privileged role as designed and allow potential risk-free MEV.

Proof of Concept

  1. User swap dai into malt and send malt directly to uniswapHandler, this is possible becuase uniswapHandler is whitelisted
    swapExactTokensForTokens(amountDai, 0, [dai.address, malt.address], uniswapHandler.address, new Date().getTime() + 10000);
  2. User send matching amount of dai to uniswapHandler
  3. User call addLiquidity() and get back LP token
  4. User call removeLiquidity() and get back both dai and malt

Recommended Mitigation Steps

According to documentation in https://github.com/code-423n4/2021-11-malt#high-level-overview-of-the-malt-protocol

Users wanting to remove liquidity can still do so via the UniswapHandler contract that is whitelisted in recovery mode.
, this should be exploitable. Meanwhile the current implementation did not actually allow remove liquidity during recovery mode (refer to issue "Unable to remove liquidity in Recovery Mode")
This exploit can be mitigated by disabling addLiquidity() when the protocol is in recovery mode

@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 8, 2021
@GalloDaSballo
Copy link
Collaborator

The Malt token is programmed to explicitly prevent buying it when at discount.
However, because addLiquidity and removeLiquidity are callable by anyone, and the UniswapHandler is whitelisted, through a calculated addition and removal of liquidity anyone can mimick buying Malt for a discount (by providing imbalanced underlying and redeeming them).

I believe this sidesteps the majority of the functionality of the protocol, and while the attack is fairly simple, it denies the protocol functionality and as such agree with a High Severity

@GalloDaSballo
Copy link
Collaborator

After re-review:
Addressing each step:

  1. The swap can be done as the whitelisting is bypassed (so finding is valid on that end)
  2. User can send more liquidity
  3. User can add liquidity at discount (as they bought at discount) (notice that this subjects them to potentially more IL so arguably the MEV extraction happened at step 1)
  4. This cannot be done because of Unable to remove liquidity in Recovery Mode #323 which was found by the same warden. As per Unable to remove liquidity in Recovery Mode #323 you can't remove liquidity as the transfer will be from pool to user and as such will be reverted due to the system being in recovery mode.

I believe that the warden identified a way to sidestep purchasing malt via purchase -> send to UniswapHandler -> add liquidity which should allow for some MEV extraction (with the user risking IL as they assume malt will go back to peg)

Because of this there's still some validity to the finding, but it's not as dire as I originally believed.

I'm going to downgrade the finding to Medium Severity as:

  1. Value can be extracted
  2. by bypassing the check for buying malt

But this doesn't allow the selling of malt, so it's not a protocol breaking exploit but rather a way to gain MEV.

@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 30, 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 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