Closed sherlock-admin closed 1 year ago
Sam underlying issue as #140.
We’ll also implement an acceptable_amount_of_bad_debt
to avoid pausing the protocol for minor amounts.
Agree that the root of this issue is the same as https://github.com/sherlock-audit/2023-06-unstoppable-judging/issues/140, but the consequence is totally different. https://github.com/sherlock-audit/2023-06-unstoppable-judging/issues/140 is more about general fund loss to the protocol, but this one is more specific to the self defensive mode.
So I would suggest keeping these 2 issue separate.
Escalate for 10 USDC The pools used in the margin dex are whitelisted and therefore highly liquid. Using a flashloan in such a pool to manipulate the reserves would not be economically viable since the flashloans are attached with a fee. It would not even be possible to repay the loan in the scenario described in the report (it would revert).
This issue should be at most medium.
PS: Note that it is not possible to sandwich other's transactions on Arbitrum. So the above assumes that a malicious user wants to sandwich its own transaction by making several function call in one transaction. I'm no MEV expert, so correct me if I'm wrong, but Arbitrum uses a sequencer to order transactions and does not have a mempool. Instead, transactions are ordered on a first come, first served basis. Therefore, it is only possible to back-run transactions. On Arbitrum MEV bots are competing for the lowest latency to the sequencer in order to identify profitable transactions and initiate a back-run.
Escalate for 10 USDC The pools used in the margin dex are whitelisted and therefore highly liquid. Using a flashloan in such a pool to manipulate the reserves would not be economically viable since the flashloans are attached with a fee. It would not even be possible to repay the loan in the scenario described in the report (it would revert).
This issue should be at most medium.
PS: Note that it is not possible to sandwich other's transactions on Arbitrum. So the above assumes that a malicious user wants to sandwich its own transaction by making several function call in one transaction. I'm no MEV expert, so correct me if I'm wrong, but Arbitrum uses a sequencer to order transactions and does not have a mempool. Instead, transactions are ordered on a first come, first served basis. Therefore, it is only possible to back-run transactions. On Arbitrum MEV bots are competing for the lowest latency to the sequencer in order to identify profitable transactions and initiate a back-run.
You've created a valid escalation!
To remove the escalation from consideration: Delete your comment.
You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.
The central focus of the report lies not in the attacker's potential profit, as at best, they break even after the repayment of the flash loan. Instead, the key concern is the attacker's ability to disrupt one of the main functionalities of the protocol at minimal cost. The attack allows the attacker to DoS the opening of new orders in the DEX, and can be repeated indefinitely, therefore the high severity of the issue.
Regarding the claim that sandwiching is not possible on Arbitrum due to the absence of a mempool, the report highlights that the attacker would be sandwiching his own closing order, making it entirely possible in Arbitrum. The attacker can achieve this by deploying a contract to obtain the flash loan, unbalance the pool, close the position on the DEX, and then repay the flash loan, all within a single transaction. As everthing is done on one single transaction, he can control the ordering of the actions, therefore his sandwich is possible.
In my escalation I brought up the impossibility to sandwich other's transactions on Arbitrum to make sure that this possibility was acknowledged by judges even for other submissions. In my escalation, I precise what you are saying regarding to doing it for your own transaction:
So the above assumes that a malicious user wants to sandwich its own transaction by making several function call in one transaction
I don't agree with what you say here:
The central focus of the report lies not in the attacker's potential profit, as at best, they break even after the repayment of the flash loan. Instead, the key concern is the attacker's ability to disrupt one of the main functionalities of the protocol at minimal cost. The attack allows the attacker to DoS the opening of new orders in the DEX, and can be repeated indefinitely, therefore the high severity of the issue.
The point of my escalation is to show that the cost is in fact not minimal. In order to grief the protocol the attacker would need to grief himself as much as the economic damage it creates (potentially more) which is not what one would call minimal cost. In order to create a situation, in a highly liquid pool, where the amount out of a trade is tiny enough to create bad debt it would require a very large capital, hence a very large fee. Therefore, creating bad debt is possible, but at a very high cost for the attacker given that he decides to provide the capital necessary for the transaction not to revert.
I think that it should be medium severity because:
Recommendation: keep the original judging.
Escalate for 10 USDC The pools used in the margin dex are whitelisted and therefore highly liquid. Using a flashloan in such a pool to manipulate the reserves would not be economically viable since the flashloans are attached with a fee. It would not even be possible to repay the loan in the scenario described in the report (it would revert).
This issue should be at most medium.
PS: Note that it is not possible to sandwich other's transactions on Arbitrum. So the above assumes that a malicious user wants to sandwich its own transaction by making several function call in one transaction. I'm no MEV expert, so correct me if I'm wrong, but Arbitrum uses a sequencer to order transactions and does not have a mempool. Instead, transactions are ordered on a first come, first served basis. Therefore, it is only possible to back-run transactions. On Arbitrum MEV bots are competing for the lowest latency to the sequencer in order to identify profitable transactions and initiate a back-run.
This issue is not about mempool, it's about intentionally causing bad debt and DOS the system, which can totally break the normal functionality at will. The defensive mode plays an important role here.
I think the vector is possible:
This is definitely a valid issue.
but I agree with Twicek, I believe the attack cost is not minimal due to existence of fees (+assuming the big flash loan to trigger the bad debt) and it also doesn't cause significant loss of funds, moreover the DOS can be lifted by admin intervention.
based on Sherlock's guideline: high, medium
I believe this as medium one, because High issue defined by definitely causes significant loss of funds.
and Significant loss of funds/large profit for the attacker at a minimal cost.
while this issue is not.
another consideration:
Could Denial-of-Service (DOS), griefing, or locking of contracts count as a Medium (or High) issue? It would not count if the DOS, etc. lasts a known, finite amount of time <1 year. If it will result in funds being inaccessible for >=1 year, then it would count as a loss of funds and be eligible for a Medium or High designation. The greater the cost of the attack for an attacker, the less severe the issue becomes.
since the DOS can be canceled by admin, a High severity might not suitable
@141345 Can you confirm my issue? #54 Being able to create bad debt means being able to extract funds from the protocol as much as bad debt.
Being able to create bad debt means being able to extract funds from the protocol as much as bad debt.
I was thinking about this one actually, as long as the attacker can create bad debt at will, the same would apply to loss amount -> "material loss of funds", and the attack can be repeat. So I still think this is a high.
So you're saying that it's a valid HIGH based on my report? If so, I think it should be a separate issue, or best report need to be change.
So you're saying that it's a valid HIGH based on my report? If so, I think it should be a separate issue, or best report need to be change.
Both https://github.com/sherlock-audit/2023-06-unstoppable-judging/issues/54 and https://github.com/sherlock-audit/2023-06-unstoppable-judging/issues/140 (same issue).
All the dups talk about abusing slippage to extract value from the protocol. I don't think it should be separate.
My reason to select 140 is, it's concise, straight to the issue, and gives an easy mitigation at the end. Some dups do not give good enough mitigation, some are not staightforward to understand.
Result:
High
Duplicate of #140
Given this issue and its duplicates and have the same underlying issue and similar attack vector as #140. The impact maybe different.
But they are to be considered duplicates.
https://docs.sherlock.xyz/audits/judging/judging#duplication-rules
Tricko
high
Attacker can DoS Margin DEX by leaving bad debt
Summary
By calling
close_trade()
with a low_min_amount_out
and sandwiching his own transaction, an attacker can create bad debt and as a consequence setself.is_accepting_new_orders = False
, thus blocking all futureopen_trade()
calls until admin intervention.Vulnerability Detail
When closing his trade, the user supplies the
_min_amount_out
argument that control the minimum amount necessary to be received from the swap in order for the call to succeed. An attacker can set a deliberately small_min_amount_out
while also sandwiching his ownclose_position
transaction to result in a very lowamount_out_received
from the swap. This enable the attacker to cause bad debt in the vault at will, makingself.is_accepting_new_orders = False
and therefore blocking new orders.By doing all in one transaction, the attacker can make sure he controls the pool state before the swap done by the
Vault
. Using the funds obtained from a flashloan he can make a big swap in the pool, making it unbalanced enough to guarantee that when theVault
swap happens, it will result in a small enoughamount_out_received
to leave bad debt (amount_out_received < position_debt_amount
).Attack reproduction steps, done in a single transaction.
open_trade()
DAI -> USDCclose_trade()
with_min_amount_out = 1
The swap done during
close_trade()
in step 4 will occur on a unabalnced DAI/USDC pool, resulting in smallamount_out_received
. The attacker can control exactly the value ofamount_out_received
by sizing the first swap done in the sandwich. Because_min_amount_out
(controlled by the attacker) is small, this swap succeds regardless of the high slippage. As a consequence theamount_out_received
is not enough to pay the position debt, making theVault
enter defensive mode (self.is_accepting_new_orders = False
) and blocking future new orders.As the
Vault
will enter defensive mode regardless of the amount of bad debt, the attacker can set this attack using small sized trades. Additionally, by assuming the role of the sandwicher during the high slippage swap, they can reclaim a substantial portion of the his margin funds. Consequently, the overall funds required for this attack remain minimal, enabling the attacker to repeat it indefinitely.Impact
The attacker can cause bad debt at will, triggering the defensive mode and blocking the DEX from accepting new orders. Even if the protocol admins enable
open_trade()
again, the attacker can keep repeating the attack to DoS the DEX indefinitely.Code Snippet
https://github.com/sherlock-audit/2023-06-unstoppable/blob/main/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L232-L279
Tool used
Manual Review
Recommendation
Consider checking if the user supplied
_min_amount_out
is equal ou above the_market_order_min_amount_out
and revert otherwise.Duplicate of #140