fix(row-permission): escape SQL values in row permission filter to prevent injection (CWE-89)

[sebastiondev]

Summary

SQL injection (CWE-89) in backend/apps/datasource/crud/row_permission.py. User-controlled values are interpolated directly into SQL WHERE clauses without escaping, allowing an authenticated non-admin user to inject arbitrary SQL via row permission filter values.

Vulnerability details

The functions transTreeItem and getSysVariableValue in row_permission.py build SQL WHERE fragments by concatenating user-controlled strings into quoted SQL literals using f-strings and .join(). None of these interpolation points escape single quotes or other SQL-significant characters.

Data flow:

1. transFilterTree() is called from get_row_permission_filters() in permission.py
2. This is invoked during data preview (preview() in datasource.py) and LLM query paths for non-admin users who have row permissions configured
3. Values reach transTreeItem() via item['enum_value'], item['value'], and user variable values — or getSysVariableValue() via current_user.name / current_user.account
4. These values are placed directly into SQL string literals like f"'{values[0]}'"
5. The constructed WHERE clause is appended to a SELECT query and executed against the datasource

Affected interpolation points (before this fix):

• enum_value list joined unsanitized into IN clause
• values[0] in LIKE clauses via f-string
• values[0] in equality/comparison via f-string
• value.split(",") in IN clauses
• current_user.name / current_user.account in getSysVariableValue()
• tree['logic'] operator — no validation, concatenated directly into the SQL fragment

Exploitability

The most realistic attack scenario involves SSO: when SSO is configured, a non-admin user controls their own display name via the identity provider. If an admin configures a row permission rule using the name or account system variable, the user's SSO name is interpolated unsanitized into the SQL query.

Preconditions:

• Authenticated non-admin user (row permissions only apply to non-admin users with id != 1)
• Admin has configured row permissions on a datasource using a system variable of type name or account
• SSO is configured and the IdP allows special characters in display names

There is an existing check_sql_read() guard in db.py that prevents write operations (INSERT/UPDATE/DELETE), but it does not prevent data exfiltration via UNION SELECT or boolean-based blind injection.

Proof of Concept

An SSO user sets their display name at the identity provider to:

test' UNION SELECT password FROM sys_user--

When the user queries any datasource table that has a row permission rule configured with the name system variable, getSysVariableValue() retrieves current_user.name and builds:

WHERE col = 'test' UNION SELECT password FROM sys_user--'

The injected UNION SELECT passes check_sql_read() (it is still a SELECT) and returns data from sys_user, including password hashes for all users.

For the enum_value and direct value paths, a user with admin access to row permission configuration could set filter values containing injection payloads — though this vector is largely redundant since admin already has full access.

The logic field can also be injected: if a crafted filter tree supplies logic: "AND 1=1) UNION SELECT ...", it would be concatenated directly into the SQL fragment without validation.

Relevant code path (current main branch):

# getSysVariableValue() in row_permission.py (line ~257)
def getSysVariableValue(sys_variable, current_user, ds, field, item):
    v = None
    if sys_variable.value[0] == 'name':
        v = current_user.name          # <-- user-controlled via SSO IdP
    # ...
    whereValue = f"'{v}'"              # <-- direct interpolation, no escaping

Fix description

This PR adds two defenses:

1. _escape_sql_value() function

A new helper that escapes string values before interpolation into SQL literals:

• Doubles single quotes (' → '') — standard SQL escaping
• Doubles backslashes (\ → \\) — prevents bypass on drivers that interpret \' as an escape sequence

Applied at every interpolation point in transTreeItem() and getSysVariableValue(), covering all value paths (enum, IN, LIKE, equality, system variables).

2. _VALID_LOGIC_OPS whitelist

The logic field from filter trees is validated against {"AND", "OR"} before use. Invalid operators cause the filter to return None (safe no-op), preventing injection via this field.

Parameterized queries would be the ideal fix, but the architecture builds SQL fragments across multiple functions and concatenates them — refactoring to parameterized queries would require significant structural changes across row_permission.py, permission.py, and datasource.py. The escaping approach is a pragmatic defense-in-depth measure that addresses the immediate injection risk.

Testing

29 unit tests added in tests/test_cwe89_escape_fix.py, all passing:

• TestEscapeSqlValue (16 tests): Validates escaping of normal strings, single quotes, double quotes, backslashes, combined payloads, UNION injection payloads, stacked queries, numeric coercion, and already-escaped input
• TestValidLogicOps (8 tests): Validates the whitelist accepts only AND/OR, rejects injection payloads, empty strings, and other operators; verifies case-insensitive matching
• TestSqlFragmentSafety (5 tests): End-to-end tests simulating how escaped values are assembled into IN, LIKE, equality, and N-prefixed SQL Server clauses, verifying that injection payloads remain inside string literals (even quote count)

$ python3 -m pytest tests/test_cwe89_escape_fix.py -v
29 passed in 0.03s

Adversarial review

Before submitting, we attempted to disprove this finding. We verified that TokenMiddleware enforces authentication on all non-whitelisted routes (the affected data preview endpoint is not whitelisted), so this requires an authenticated session. We checked whether the transFilterTerm() function could serve as an injection vector — it cannot, as it maps item['term'] through a fixed if/elif chain that returns only hardcoded SQL operators. We also considered whether most attack vectors are redundant because they require admin access to configure — they are, but the SSO user name path through getSysVariableValue() is controlled by the non-admin user themselves and represents a genuine escalation path. The fix still hardens all paths as defense-in-depth.

---

_{Submitted by Sebastion — autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.}

[sebastiondev]

Thanks for the merge. Glad it landed cleanly.

P.S. Sebastion is run by Foundation Machines if ongoing autonomous audits would be useful.