fix(polardb): escape user-supplied values in edge_exists() to prevent Cypher injection (CWE-89)#1637
Open
sebastiondev wants to merge 2 commits intoMemTensor:mainfrom
Open
Conversation
…njection (CWE-89) The edge_exists() method in the PolarDB graph database module interpolated source_id, target_id, user_name, and type directly into Cypher query strings via f-strings without any sanitization. An attacker providing a crafted value such as source_id="x' OR 1=1 --" could manipulate the Cypher WHERE clause to bypass authorization checks or extract unauthorized data. Fix: escape all four user-supplied values via the existing escape_sql_string() function (which doubles single quotes) before interpolating them into the Cypher query, consistent with how other methods in the same file (get_edges, _build_user_name_and_kb_ids_conditions_cypher) already handle escaping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a Cypher injection vulnerability in
PolarDBGraphDB.edge_exists()(CWE-89). User-supplied values (source_id,target_id,user_name,type) were interpolated directly into a Cypher query string with no escaping, allowing an attacker to break out of string literals and modify the executed query.src/memos/graph_dbs/polardb.pyPolarDBGraphDB.edge_exists()The vulnerability
The pre-fix query was constructed like this:
The Cypher body is passed to Postgres/AGE inside a
$$ ... $$dollar-quoted string. Because of the dollar quoting,psycopg2placeholders cannot reach the Cypher string literals — parameter binding happens at the Postgres layer, not inside the Cypher payload. So the only correct mitigation is to escape single quotes in the values before they are embedded.A value like
x' OR 1=1 //supplied assource_idcloses the Cypher string, injects an additional clause, and comments out the rest. The same applies totarget_id,user_name, andtype.Reaching the sink with attacker input
user_nameis taken from theX-User-NameHTTP header via the request-context middleware (src/memos/api/middleware/request_context.py) and consumed downstream by graph operations.source_id/target_idflow in from API memory/edge endpoints. In a default deployment the header is trusted with no strong authentication binding it to the caller, so the value reachesedge_exists()directly under attacker control.The fix
Escape all user-supplied string values with the existing
escape_sql_string()helper (which doubles single quotes — the standard Cypher/SQL string-literal escape) before embedding them in the dollar-quoted Cypher block.directionwas already validated against a whitelist;type, when not"ANY", is now also escaped.The change is minimal and matches the escaping pattern already used elsewhere in
polardb.py.Tests
A new test module
tests/graph_dbs/test_polardb_edge_exists_cwe89.pywas added with 11 cases covering:source_idis doubled, raw payload not present in querytarget_idis doubleduser_nameis doubled (both occurrences)typevalue is escaped when supplieddirectionwhitelist still rejects unknown values withValueErrorANYdirection andANYtype still produce a valid queryuser_namefalls back toself.config.user_namecorrectlyRun:
A small
tests/graph_dbs/conftest.pywas added so the tests can importmemoswithout the full env.Adversarial review
Before submitting, we tried to disprove this. We checked whether
psycopg2parameterization could safely cover the values — it can't, because the Cypher payload sits inside a$$ ... $$dollar-quoted Postgres string, so the inner literals are opaque to the driver. We checked whether upstream callers sanitize the IDs oruser_name— they don't;user_namein particular comes from theX-User-Namerequest header and is forwarded as-is. We checked whether an alternate code path (edge_exists_old) makes the fix redundant — it's parameterized but isn't the one in use. The exploit precondition is simply "caller can set request headers and the PolarDB graph backend is configured", which is the normal operating mode.Scope
Only
edge_exists()is changed. Other functions in the same file have separate handling and are out of scope for this PR.cc @lewiswigmore