Skip to content

Commit 56f19b8

Browse files
author
Saurabh Badenkal
committed
Fix CodeQL ReDoS: replace vulnerable write regex with comment-stripping approach
CodeQL found exponential backtracking in _SQL_WRITE_RE which used nested quantifiers for SQL comment matching: (?:/\*.*?\*/\s*|...)* Fix: separate comment stripping (_SQL_COMMENT_RE) from write detection. 1. Strip SQL comments with a safe non-backtracking regex 2. Check for write keywords with the simple anchored regex The comment regex uses [^*]*\*+(?:[^/*][^*]*\*+)*/ which is the standard safe pattern for matching C-style block comments. 3 new tests for comment-prefixed write detection. 774 unit tests passing.
1 parent b5eb12b commit 56f19b8

2 files changed

Lines changed: 23 additions & 6 deletions

File tree

src/PowerPlatform/Dataverse/data/_odata.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -839,9 +839,10 @@ def _do_request(url: str, *, params: Optional[Dict[str, Any]] = None) -> Dict[st
839839

840840
# ----------------------- SQL guardrail patterns --------------------
841841
_SQL_WRITE_RE = re.compile(
842-
r"^\s*(?:/\*.*?\*/\s*|--[^\n]*\n\s*)*(?:INSERT|UPDATE|DELETE|DROP|TRUNCATE|ALTER|CREATE|EXEC|GRANT|REVOKE|BULK)\b",
843-
re.IGNORECASE | re.DOTALL,
842+
r"^\s*(?:INSERT|UPDATE|DELETE|DROP|TRUNCATE|ALTER|CREATE|EXEC|GRANT|REVOKE|BULK)\b",
843+
re.IGNORECASE,
844844
)
845+
_SQL_COMMENT_RE = re.compile(r"/\*[^*]*\*+(?:[^/*][^*]*\*+)*/|--[^\n]*", re.DOTALL)
845846
_SQL_LEADING_WILDCARD_RE = re.compile(r"\bLIKE\s+'%[^']", re.IGNORECASE)
846847
_SQL_IMPLICIT_CROSS_JOIN_RE = re.compile(
847848
r"\bFROM\s+[A-Za-z0-9_]+(?:\s+[A-Za-z0-9_]+)?\s*,\s*[A-Za-z0-9_]+",
@@ -929,8 +930,9 @@ def _sql_guardrails(self, sql: str) -> str:
929930
"""
930931
# --- BLOCKED (save server round-trip) ---
931932

932-
# 1. Block writes
933-
if self._SQL_WRITE_RE.search(sql):
933+
# 1. Block writes (strip SQL comments first to catch comment-prefixed writes)
934+
sql_no_comments = self._SQL_COMMENT_RE.sub(" ", sql).strip()
935+
if self._SQL_WRITE_RE.search(sql_no_comments):
934936
raise ValidationError(
935937
"SQL endpoint is read-only. Use client.records or "
936938
"client.dataframe for write operations "
@@ -1035,8 +1037,10 @@ def _query_sql(self, sql: str) -> list[dict[str, Any]]:
10351037
sql = sql.strip()
10361038

10371039
# Block write statements FIRST (before table extraction, since
1038-
# UPDATE/INSERT/DELETE don't have FROM clauses)
1039-
if self._SQL_WRITE_RE.search(sql):
1040+
# UPDATE/INSERT/DELETE don't have FROM clauses).
1041+
# Strip SQL comments to catch e.g. /**/DELETE or --\\nDELETE.
1042+
sql_no_comments = self._SQL_COMMENT_RE.sub(" ", sql).strip()
1043+
if self._SQL_WRITE_RE.search(sql_no_comments):
10401044
raise ValidationError(
10411045
"SQL endpoint is read-only. Use client.records or "
10421046
"client.dataframe for write operations "

tests/unit/data/test_sql_guardrails.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,19 @@ def test_select_with_delete_in_where_not_blocked(self):
7878
result = c._sql_guardrails("SELECT TOP 10 name FROM account WHERE name = 'DELETE ME'")
7979
assert "SELECT" in result
8080

81+
@pytest.mark.parametrize(
82+
"sql",
83+
[
84+
"/* comment */ DELETE FROM account",
85+
"-- line comment\nDELETE FROM account",
86+
"/* multi\nline */ DROP TABLE account",
87+
],
88+
)
89+
def test_comment_prefixed_writes_blocked(self, sql):
90+
c = _client()
91+
with pytest.raises(ValidationError, match="read-only"):
92+
c._sql_guardrails(sql)
93+
8194

8295
# ===================================================================
8396
# 1b. Block server-rejected SQL patterns (save round-trip)

0 commit comments

Comments
 (0)