Skip to content

Conversation

@kaze-cow
Copy link

@kaze-cow kaze-cow commented Oct 22, 2025

Description

We recently learned that due to Balancer Deployer wallet being compromised due to the profanity vanity address generator vulnerability.

As we are planning to continue deploying on these networks with the same deployer address, we wanted to disclose this as a known issue prior to deployment on these affected networks.

Changes

  • Added a description to the Known Issues section describing the vulnerability.

Summary by CodeRabbit

  • Documentation
    • Added a Known Issues/Security subsection describing a vulnerability where a compromised Balancer vault deployer can cause trade DoS or fund loss. The section outlines affected networks, conditions for harm, root cause, settlement behavior, observable effects, exploitability criteria, and includes a warning advising against signing orders that use BALANCE_INTERNAL for buyTokenBalance or sellTokenBalance.

…lity

We recently learned that due to Balancer Deployer wallet being compromised due to the profanity vanity address generator vulnerability.

As we are planning to continue deploying on these networks with the same deployer address, we wanted to disclose this as a known issue prior to deployment on these affected networks.
@kaze-cow kaze-cow self-assigned this Oct 22, 2025
@kaze-cow kaze-cow requested a review from a team as a code owner October 22, 2025 06:26
@vercel
Copy link

vercel bot commented Oct 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
docs Ready Ready Preview Oct 30, 2025 9:11am

@github-actions
Copy link

github-actions bot commented Oct 22, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Walkthrough

Added a documentation subsection to the CoW Protocol contracts reference describing a vulnerability scenario where a compromised Balancer vault (used with BALANCE_INTERNAL) can enable DoS of trade batches or theft of user funds, including prerequisites, affected networks, and a warning against using BALANCE_INTERNAL for signed balances.

Changes

Cohort / File(s) Summary
Security documentation
docs/cow-protocol/reference/contracts/core/README.mdx
Added subsection documenting a vulnerability scenario for compromised Balancer vault deployer addresses when using BALANCE_INTERNAL, describing root cause, settlement/CREATE2 implications, observable effects (trade reversion prevention and potential fund theft), exploitability prerequisites, affected networks, and a warning advising against signing orders with BALANCE_INTERNAL.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Primary file to review: docs/cow-protocol/reference/contracts/core/README.mdx

Poem

🐰 In the docs I nibble, a caution I pen,

A vault gone awry could foul trades of men,
BALANCE_INTERNAL — a path that's thin,
Sign not in the dark, keep your balance in. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "doc(contracts/core): disclose compromised balancer contract vulnerability" directly and accurately reflects the main change in the changeset, which adds a new documentation section to the Known Issues describing a vulnerability scenario involving a compromised Balancer Vault deployer address. The title is concise, specific, and clearly communicates the primary purpose of the change without vague language or extraneous details. A reviewer scanning the repository history would immediately understand that this PR discloses a security vulnerability in the documentation.
Description Check ✅ Passed The pull request description follows the repository's template structure with both required sections properly completed. The Description section explains the context and rationale for the change (the compromised Balancer Deployer wallet and the need to disclose this as a known issue before deployment), and the Changes section clearly lists what was added with a completed checkbox. While the Changes section contains only one item rather than an exhaustive list of all documentation details, this is appropriate for a documentation-only PR where a single consolidated change is the most relevant unit of work.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch doc/disclose-balancer

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kaze-cow kaze-cow requested a review from fedgiac October 22, 2025 06:26
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/cow-protocol/reference/contracts/core/README.mdx (1)

170-170: Simplify phrasing for conciseness.

Three phrases can be tightened per standard style guidance:

  • Line 170: "In order to preserve" → "To preserve"
  • Line 182: "outside of the usual" → "outside the usual"
  • Line 188: "In order to avoid" → "To avoid"
-In order to preserve a consistent settlement address on all chains, CoW Protocol uses the `CREATE2` opcode to deploy the settlement contract. Since the balancer vault address is supplied as an immutable constructor property, the Settlement contract contains a dependency upon this compromised contract.
+To preserve a consistent settlement address on all chains, CoW Protocol uses the `CREATE2` opcode to deploy the settlement contract. Since the balancer vault address is supplied as an immutable constructor property, the Settlement contract contains a dependency upon this compromised contract.
-3. A malicious solver must obtain the signed order outside of the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).
+3. A malicious solver must obtain the signed order outside the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).
-:::warning
-
-In order to avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
-
-:::
+:::warning
+
+To avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
+
+:::

Also applies to: 182-182, 188-188

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1c99ed and d58e886.

📒 Files selected for processing (1)
  • docs/cow-protocol/reference/contracts/core/README.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/cow-protocol/reference/contracts/core/README.mdx

[style] ~170-~170: Consider a more concise word here.
Context: ...s://docs.balancer.fi/concepts/vault/). In order to preserve a consistent settlement addres...

(IN_ORDER_TO_PREMIUM)


[style] ~182-~182: This phrase is redundant. Consider using “outside”.
Context: ...ous solver must obtain the signed order outside of the usual order flow (the CoW Orderbook...

(OUTSIDE_OF)


[style] ~188-~188: Consider a more concise word here.
Context: ...is vulnerability: * Plasma :::warning In order to avoid being affected by this vulnerabil...

(IN_ORDER_TO_PREMIUM)


[grammar] ~189-~189: Ensure spelling is correct
Context: ...fected by this vulnerability, we do not reccomend signing orders with buyTokenBalance o...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments (1)
docs/cow-protocol/reference/contracts/core/README.mdx (1)

165-189: LGTM for security disclosure; minor corrections needed.

The vulnerability disclosure is clear, well-structured, and appropriately scoped. The explanation properly:

  • Describes the root cause (profanity vulnerability in address generator)
  • Articulates the attack vectors (DoS and fund theft)
  • Lists the three prerequisite conditions for exploitation
  • Provides clear mitigation guidance via warning block

The documentation strike the right balance between technical accuracy and accessibility. After addressing the spelling error and optional style improvements above, this is ready to merge.

Verify that the list of affected networks (currently "Plasma" only) is accurate and complete as of the PR date.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/cow-protocol/reference/contracts/core/README.mdx (1)

170-170: Apply style improvements for conciseness.

Three instances of verbose phrasing can be simplified:

  1. Line 170: "In order to preserve" → "To preserve"
  2. Line 182: "outside of the usual order flow" → "outside the usual order flow"
  3. Line 188: "In order to avoid" → "To avoid"

Apply this diff:

- In order to preserve a consistent settlement address on all chains, CoW Protocol uses the `CREATE2` opcode to deploy the settlement contract. Since the balancer vault address is supplied as an immutable constructor property, the Settlement contract contains a dependency upon this compromised contract.
+ To preserve a consistent settlement address on all chains, CoW Protocol uses the `CREATE2` opcode to deploy the settlement contract. Since the balancer vault address is supplied as an immutable constructor property, the Settlement contract contains a dependency upon this compromised contract.
- 3. A malicious solver must obtain the signed order outside of the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).
+ 3. A malicious solver must obtain the signed order outside the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).
- In order to avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
+ To avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.

Also applies to: 182-182, 188-188

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d58e886 and 7e078e4.

📒 Files selected for processing (1)
  • docs/cow-protocol/reference/contracts/core/README.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/cow-protocol/reference/contracts/core/README.mdx

[style] ~170-~170: Consider a more concise word here.
Context: ...s://docs.balancer.fi/concepts/vault/). In order to preserve a consistent settlement addres...

(IN_ORDER_TO_PREMIUM)


[style] ~182-~182: This phrase is redundant. Consider using “outside”.
Context: ...ous solver must obtain the signed order outside of the usual order flow (the CoW Orderbook...

(OUTSIDE_OF)


[style] ~188-~188: Consider a more concise word here.
Context: ...is vulnerability: * Plasma :::warning In order to avoid being affected by this vulnerabil...

(IN_ORDER_TO_PREMIUM)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Socket Security: Pull Request Alerts
  • GitHub Check: Socket Security: Project Report
🔇 Additional comments (1)
docs/cow-protocol/reference/contracts/core/README.mdx (1)

165-191: Documentation disclosure is thorough and accurate.

The vulnerability description accurately reflects the Profanity weakness involving a 32-bit seed for 256-bit private keys, enabling compromise of addresses generated with the tool. The subsection clearly outlines the attack prerequisites, possible consequences (DoS or theft), and actionable mitigation (avoiding BALANCE_INTERNAL).

The technical context around CREATE2 deterministic deployment and immutable Balancer vault addresses is well-explained. The warning is appropriately prominent and specific.

@kaze-cow
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Oct 30, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
docs/cow-protocol/reference/contracts/core/README.mdx (3)

170-171: Simplify "In order to" → "To".

Tighter phrasing improves readability.

-In order to preserve a consistent settlement address on all chains, CoW Protocol uses the `CREATE2` opcode to deploy the settlement contract. Since the balancer vault address is supplied as an immutable constructor property, the Settlement contract contains a dependency upon this compromised contract.
+To preserve a consistent settlement address on all chains, CoW Protocol uses the `CREATE2` opcode to deploy the settlement contract. Since the balancer vault address is supplied as an immutable constructor property, the Settlement contract contains a dependency upon this compromised contract.

182-182: Simplify "outside of" → "outside".

Removes redundant preposition.

-3. The malicious solver must obtain the signed order outside of the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).
+3. The malicious solver must obtain the signed order outside the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).

188-189: Simplify "In order to" → "To".

Tighter phrasing improves readability.

 :::warning
 
-In order to avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
+To avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e078e4 and f4b029f.

📒 Files selected for processing (1)
  • docs/cow-protocol/reference/contracts/core/README.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/cow-protocol/reference/contracts/core/README.mdx

[grammar] ~167-~167: Use a hyphen to join words.
Context: ...e user initiates a trade through a third party endpoint submitting a order type t...

(QB_NEW_EN_HYPHEN)


[style] ~170-~170: Consider a more concise word here.
Context: ...s://docs.balancer.fi/concepts/vault/). In order to preserve a consistent settlement addres...

(IN_ORDER_TO_PREMIUM)


[style] ~182-~182: This phrase is redundant. Consider using “outside”.
Context: ...ous solver must obtain the signed order outside of the usual order flow (the CoW Orderbook...

(OUTSIDE_OF)


[style] ~188-~188: Consider a more concise word here.
Context: ...is vulnerability: * Plasma :::warning In order to avoid being affected by this vulnerabil...

(IN_ORDER_TO_PREMIUM)


### Loss of funds if a compromised Balancer contract is deployed

When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting a order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix article agreement: use "an" instead of "a" before vowel sound.

Line 167 uses "a order type" which is grammatically incorrect.

-When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting a order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
+When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting an order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting a order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting an order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
🧰 Tools
🪛 LanguageTool

[grammar] ~167-~167: Use a hyphen to join words.
Context: ...e user initiates a trade through a third party endpoint submitting a order type t...

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents
In docs/cow-protocol/reference/contracts/core/README.mdx around line 167, the
phrase "submitting a order type" uses the incorrect article "a" before a vowel
sound; change it to "submitting an order type" so the sentence reads correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make this change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pretf00d, great! Here's the corrected line:

When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting an order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.

The article has been changed from "a" to "an" before "order type" to maintain proper grammar.

Comment on lines +181 to +182
2. The trade must have been signed with `buyTokenBalance` or `sellTokenBalance` as `BALANCE_INTERNAL`. As of writing, this order type has not been processed by the protocol since March 2024.
3. The malicious solver must obtain the signed order outside of the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify factual claims: March 2024 date, 1.5m USDC bond amount, and affected networks list.

The documentation cites specific dates and bond amounts that should be confirmed for accuracy. Additionally, "Plasma" is listed as the only affected network; please confirm this list is complete.


🏁 Script executed:

#!/bin/bash
# Search for references to the March 2024 cutoff date for BALANCE_INTERNAL orders and the current bond amount
# to verify these claims are documented elsewhere in the codebase or linked issues.

# Look for references to BALANCE_INTERNAL in the codebase
rg -n "BALANCE_INTERNAL" --type=ts --type=jsx --type=mdx -i

# Search for bond amount context
rg -n "1.5" --type=mdx -C2

# Check for other security or issue discussions that might reference these specifics
rg -n "March 2024" --type=mdx -C2

Length of output: 83


Let me generate corrected shell scripts to work around the file type limitation:

Based on the web search results, I found important information about CoW Protocol deployments and networks. Let me search the codebase to verify the specific claims in the documentation:Based on my verification through web searches, I have found important information:

Key Finding: CoW Protocol requires each solver to deposit $500,000 worth of cUSDC and 1.5M COW tokens as collateral. This relates to the bond amount mentioned, though the documentation cites it as "1.5m USDC" which appears to be imprecise—it's actually 1.5M COW tokens, not USDC.

Critical Issue: Multiple web searches confirm CoW Protocol has deployed on Ethereum, Gnosis Chain, Arbitrum One, and Base. Notably, Plasma is not listed as an affected network in any official documentation. References to "Plasma" in search results refer to it as a general Ethereum scaling solution framework, not an active CoW Protocol deployment.

The documentation claims "Plasma" is the only affected network for BALANCE_INTERNAL vulnerabilities, but the web search results do not support Plasma as a current CoW Protocol deployment.


Verify and correct the network list and bond amount specificity.

The documentation should clarify:

  1. Bond amount: The solver collateral requirement is 1.5M COW tokens (not USDC) as collateral, distinct from the $500,000 cUSDC requirement.
  2. Affected networks: Replace "Plasma" with the actual networks where CoW Protocol operates. Current deployments are Ethereum, Gnosis Chain, and Arbitrum One.
  3. March 2024 claim: Search results did not definitively confirm the March 2024 cutoff date for BALANCE_INTERNAL orders; verify this date against GitHub issues or protocol changelog.

🏁 Script executed:

#!/bin/bash
# Find and search .mdx files for the specific claims
# Search for BALANCE_INTERNAL references
fd -e mdx --exec grep -n "BALANCE_INTERNAL" {} +

# Search for bond amount references (1.5m, 1500000)
fd -e mdx --exec grep -n -E "(1\.5\s*m|1500000|bond|collateral)" {} + | head -30

# Search for March 2024 references
fd -e mdx --exec grep -n "March 2024\|2024.*[Mm]arch" {} +

# Search for Plasma network references
fd -e mdx --exec grep -n -i "plasma\|network" {} + | grep -i plasma | head -20

Length of output: 9924


🏁 Script executed:

#!/bin/bash
# Search for GitHub issues or discussions that might reference the bond amount, March 2024 cutoff
# This helps verify if these are documented consensus points

# Look for any linked issue references in .mdx files
fd -e mdx --exec grep -n -E "(issue|#[0-9]+|github\.com)" {} + | head -30

Length of output: 10452


Based on my verification of the codebase and web searches, I've identified critical inconsistencies:

Critical Issue Found: Plasma Network Listing

The documentation at line 185 lists "Plasma" as the only affected network for the BALANCE_INTERNAL vulnerability. However, web search results confirm CoW Protocol is deployed on: Ethereum, Gnosis Chain, Arbitrum One, Base, Avalanche, Polygon, Lens, and BNB. Plasma is not listed as an active CoW Protocol deployment in any official sources.

Issues Verified:

  1. Plasma network is incorrectly listed — It does not appear as a CoW Protocol deployment target
  2. Bond amount description: The documentation correctly cites that solvers post bonds, though web sources confirm these are enforced by CoW DAO but don't clarify the specific USDC amount
  3. March 2024 cutoff: This date is documented in the codebase at line 181 but cannot be independently confirmed via web search

Remove or correct the Plasma network reference; verify against official CoW Protocol deployment list.

Line 185 should be updated to reflect actual networks where CoW Protocol operates. If BALANCE_INTERNAL vulnerability exists on specific networks, cross-reference against the contract deployments documented elsewhere (Ethereum mainnet, Gnosis chain, Arbitrum one, Base, Avalanche, Polygon, Lens, BNB, Sepolia). Additionally, manually verify the March 2024 cutoff date against GitHub issues or protocol changelogs to ensure accuracy of timeline claims.

🧰 Tools
🪛 LanguageTool

[style] ~182-~182: This phrase is redundant. Consider using “outside”.
Context: ...ous solver must obtain the signed order outside of the usual order flow (the CoW Orderbook...

(OUTSIDE_OF)

Comment on lines +184 to +185
As of writing, these are the CoW-supported networks affected by this vulnerability:
* Plasma
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating with latest findings plus fix chains. Plasma is good.

Suggested change
As of writing, these are the CoW-supported networks affected by this vulnerability:
* Plasma
As of this writing, there are no official settlement contract deployments on chains where the Balancer vault can be used to compromise user orders that don't use the Balancer flow (case 1).
In any case, note that CoW orders are always with `feeAmount = 0`, which would further limit the impact of this issue.
However, the following networks are currently at risk of loss of funds for user order intents that explicitly request trading through the Balancer vault (case 2):
* Linea
* Lens
Note that trading through the Balancer vault is currently unsupported at the protocol level and compromised orders cannot be posted to the orderbook.


:::warning

In order to avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BALANCE_INTERNAL isn't what the user sees when signing, but rather internal. Also, external is also bad.

Suggested change
In order to avoid being affected by this vulnerability, we do not recommend signing orders with `buyTokenBalance` or `sellTokenBalance` set to `BALANCE_INTERNAL`.
In order to avoid being affected by this vulnerability, we recommend only signing orders with `buyTokenBalance` or `sellTokenBalance` set to `erc20`.


### Loss of funds if a compromised Balancer contract is deployed

When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting a order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating to include the most recent findings.

Suggested change
When a user is trading on certain affected networks and the user initiates a trade through a third party endpoint submitting a order type that has not been supported since 2024, it is possible that a malicious solver could steal the entire input amount of a user's trade.
If the settlement contract deployment points to an absent or compromised Balancer vault, user orders may be seriously affected.

Comment on lines +175 to +177
There are two general effects that this vulnerability can have:
* If the compromised contract at the `Balancer` address reverts upon a call to `manageUserBalance`, the attacker can effectively prevent users from trading in any trade batches that contain a trade using `BALANCE_INTERNAL`.
* If the compromised contract does *not* revert, the settlement contract assumes that the user's balance was successfully transferred, and no further action is taken. This assumption allows for a solver to steal the full amount of a user's trade output, since the settlement contract does not verify that the funds were received by the user.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating to include the most recent findings. I wouldn't bother mentioning "user can't trade" because there's no Balancer vault there's no reason for the user to want to trade through it.

Suggested change
There are two general effects that this vulnerability can have:
* If the compromised contract at the `Balancer` address reverts upon a call to `manageUserBalance`, the attacker can effectively prevent users from trading in any trade batches that contain a trade using `BALANCE_INTERNAL`.
* If the compromised contract does *not* revert, the settlement contract assumes that the user's balance was successfully transferred, and no further action is taken. This assumption allows for a solver to steal the full amount of a user's trade output, since the settlement contract does not verify that the funds were received by the user.
There are two levels for this vulnerability:
1. The target vault is specifically designed to exploit CoW user orders.
2. The vault is empty or uses a contract that isn't designed to perform this specific exploit.
In case 1, the risks for the users are:
- Malicious solvers can steal `feeAmount` from *any existing user order*.
- Malicious solvers can make an order appear filled despite it not having been executed. This is particularly bad for specific classes of smart-contract orders relying on the filled amount of an order in their internal logic, as for example the ETH flow.
- All risks for case 2.
In case 2, the risks for the users are:
- If the user signs an order using Balancer as the balance source/target for the order (i.e., using `internal` or `external` for `buyTokenBalance` or `sellTokenBalance`) then a solver may be able to steal the full amount of a user's trade output: the settlement contract assumes that the user's balance was successfully transferred from/to the vault, and no further action is taken.

Comment on lines +179 to +183
In total, there are 3 conditions that are needed to make this vulnerability harmful:
1. The user must be trading on a network with a compromised Balancer contract (see affected networks below).
2. The trade must have been signed with `buyTokenBalance` or `sellTokenBalance` as `BALANCE_INTERNAL`. As of writing, this order type has not been processed by the protocol since March 2024.
3. The malicious solver must obtain the signed order outside of the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately 2 and 3 don't hold anymore in the general case. 1 is already mentioned before so I'd drop the paragraph.

Suggested change
In total, there are 3 conditions that are needed to make this vulnerability harmful:
1. The user must be trading on a network with a compromised Balancer contract (see affected networks below).
2. The trade must have been signed with `buyTokenBalance` or `sellTokenBalance` as `BALANCE_INTERNAL`. As of writing, this order type has not been processed by the protocol since March 2024.
3. The malicious solver must obtain the signed order outside of the usual order flow (the CoW Orderbook API rejects these order types) and steal the user's funds. For the attack to be profitable, the stolen amount must exceed the current bond amount (as of writing, 1.5m USDC).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants