This repository has been archived by the owner on Jun 30, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5
0xpep7 - Share Price Inflation by First LP-er, Enabling DOS Attacks on Subsequent buyShares with Up to 1001x the Attacking Cost #55
Labels
Has Duplicates
A valid issue with 1+ other issues describing the same vulnerability
Medium
A valid Medium severity issue
Reward
A payout will be made for this issue
Sponsor Confirmed
The sponsor acknowledged this issue is valid
Will Fix
The sponsor confirmed this issue will be fixed
Comments
sherlock-admin
changed the title
Magnificent Vanilla Capybara - mtFee can get locked forever if the MAINTAINER address is blacklisted by one of the tokens of a pool
Suave Jetblack Kookaburra - Share Price Inflation by First LP-er, Enabling DOS Attacks on Subsequent buyShares with Up to 1001x the Attacking Cost
Dec 28, 2023
github-actions
bot
added
High
A valid High severity issue
Has Duplicates
A valid issue with 1+ other issues describing the same vulnerability
labels
Jan 2, 2024
This was referenced Jan 2, 2024
Closed
Skyewwww
added
Sponsor Confirmed
The sponsor acknowledged this issue is valid
Will Fix
The sponsor confirmed this issue will be fixed
labels
Jan 5, 2024
Skyewwww
added a commit
to DODOEX/dodo-gassaving-pool
that referenced
this issue
Jan 10, 2024
This was referenced Jan 10, 2024
sherlock-admin
changed the title
Suave Jetblack Kookaburra - Share Price Inflation by First LP-er, Enabling DOS Attacks on Subsequent buyShares with Up to 1001x the Attacking Cost
0xpep7 - Share Price Inflation by First LP-er, Enabling DOS Attacks on Subsequent buyShares with Up to 1001x the Attacking Cost
Jan 12, 2024
We have fixed this bug at this PR: DODOEX/dodo-gassaving-pool#14 |
Because of the fact that this is "just" DoS (no loss of funds) and there are serious constraints on whether the attack is possible and there are high capital requirements for performing it, will make it a valid Medium. |
sherlock-admin
added
Non-Reward
This issue will not receive a payout
and removed
Reward
A payout will be made for this issue
labels
Jan 22, 2024
Czar102
added
Medium
A valid Medium severity issue
Reward
A payout will be made for this issue
and removed
Non-Reward
This issue will not receive a payout
labels
Jan 22, 2024
Fix looks good |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
Has Duplicates
A valid issue with 1+ other issues describing the same vulnerability
Medium
A valid Medium severity issue
Reward
A payout will be made for this issue
Sponsor Confirmed
The sponsor acknowledged this issue is valid
Will Fix
The sponsor confirmed this issue will be fixed
0xpep7
medium
Share Price Inflation by First LP-er, Enabling DOS Attacks on Subsequent buyShares with Up to 1001x the Attacking Cost
Summary
The smart contract contains a critical vulnerability that allows a malicious actor to manipulate the share price during the initialization of the liquidity pool, potentially leading to a DOS attack on subsequent buyShares operations.
Vulnerability Detail
The root cause of the vulnerability lies in the initialization process of the liquidity pool, specifically in the calculation of shares during the first deposit.
If the pool is empty, the smart contract directly sets the share value based on the minimium value of the base token denominated value of the provided assets. This assumption can be manipulated by a malicious actor during the first deposit, leading to a situation where the LP pool token becomes extremely expensive.
Attack Scenario
The attacker exploits the vulnerability during the initialization of the liquidity pool:
The attacker mints 1001 shares during the first deposit.
Immediately, the attacker sells back 1000 shares, ensuring to keep 1 wei via the
sellShares
function.The attacker then donates a large amount (1000e18) of base and quote tokens and invokes the
sync()
routine to pump the base and quote reserves to 1001 + 1000e18.The protocol users proceed to execute the
buyShares
function with a balance less thanattacker's spending * 1001
. The transaction reverts due to themintRatio
being kept below 1001 wad and the computedshares
less than 1001 (line 71), while it needs a value >= 1001 to mint shares successfully._mint()
function fails with a "MINT_AMOUNT_NOT_ENOUGH" error, causing a denial-of-service condition for subsequent buyShares operations.POC
Apply the POC to
dodo-gassaving-pool/test/GPSTrader.t.sol
and run withcd dodo-gassaving-pool && forge test --fork-url "https://rpc.flashbots.net" -vvv --mt test_mint1weiShares_DOSx1000DonationVolume
to check the result.A PASS result would confirm that any deposits with volume less than 1001 times to attacker cost would fail. That means by spending $1000, the attacker can DOS any transaction with volume below $1001,000.
Impact
The impact of this vulnerability is severe, as it allows an attacker to conduct DOS attacks on buyShares with a low attacking cost (retrievable for further attacks via
sellShares
). This significantly impairs the core functionality of the protocol, potentially preventing further LP operations and hindering the protocol's ability to attract Total Value Locked (TVL) for other trading operations such as sellBase, sellQuote and flashloan.Code Snippet
https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L56-L65
Tool used
Foundry test
Recommendation
A mechanism should be implemented to handle the case of zero totalSupply during initialization. A potential solution is inspired by Uniswap V2 Core Code, which sends the first 1001 LP tokens to the zero address. This way, it's extremely costly to inflate the share price as much as 1001 times on the first deposit.
// File: dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol 31: function buyShares(address to) ... 56: if (totalSupply == 0) { 57: // case 1. initial supply 58: // The shares will be minted to user 59: shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) 60: ? DecimalMath.divFloor(quoteBalance, _I_) 61: : baseBalance; + _mint(address(0), 1001); // permanently lock the first MINIMUM_LIQUIDITY of 1001 tokens, makes it imposible to manipulate the totalSupply to 1 wei ... ``` ...
The text was updated successfully, but these errors were encountered: