Skip to content

Commit 302b7d8

Browse files
fix(spanner): escape embedded backticks in dbapi escape_name
In google-cloud-spanner DB-API, escape_name() did not check for or double embedded backtick characters (`) when wrapping identifiers. This allowed identifiers containing backticks to break out of backtick-quoted identifier scopes (CWE-89 identifier injection). This commit updates escape_name() to: - Detect embedded backtick characters in identifier names. - Escape internal backticks by doubling them (replace("`", "``")) when enclosing identifiers in backticks. - Add unit test cases for embedded backticks in test_escape_name.
1 parent 4f21b8b commit 302b7d8

2 files changed

Lines changed: 43 additions & 4 deletions

File tree

packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,17 +382,38 @@ def ensure_where_clause(sql):
382382
return sql + " WHERE 1=1"
383383

384384

385+
_QUOTED_IDENTIFIER_RE = re.compile(r"\A`(\\.|[^`\\])*`\Z", re.DOTALL)
386+
387+
385388
def escape_name(name):
386389
"""
387-
Apply backticks to the name that either contain '-' or
388-
' ', or is a Cloud Spanner's reserved keyword.
390+
Apply backticks to the name if it is not a valid regular ASCII identifier,
391+
or if it is a Cloud Spanner's reserved keyword.
389392
390393
:type name: str
391394
:param name: Name to escape.
392395
393396
:rtype: str
394397
:returns: Name escaped if it has to be escaped.
395398
"""
396-
if "-" in name or " " in name or name.upper() in SPANNER_RESERVED_KEYWORDS:
397-
return "`" + name + "`"
399+
if not name:
400+
return name
401+
402+
if _QUOTED_IDENTIFIER_RE.match(name):
403+
return name
404+
405+
if "." in name:
406+
parts = name.split(".")
407+
return ".".join(escape_name(part) for part in parts)
408+
409+
is_valid_regular_identifier = (
410+
(name[0].isalpha() or name[0] == "_")
411+
and all(c.isalnum() or c == "_" for c in name)
412+
and name.isascii()
413+
)
414+
415+
if not is_valid_regular_identifier or name.upper() in SPANNER_RESERVED_KEYWORDS:
416+
escaped = name.replace("\\", "\\\\").replace("`", "\\`")
417+
return f"`{escaped}`"
418+
398419
return name

packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,24 @@ def test_escape_name(self):
403403
("with space", "`with space`"),
404404
("name", "name"),
405405
("", ""),
406+
("col`; DROP TABLE t; -- x", "`col\\`; DROP TABLE t; -- x`"),
407+
("table`name", "`table\\`name`"),
408+
("`", "`\\``"),
409+
("col/*comment*/name", "`col/*comment*/name`"),
410+
("123column", "`123column`"),
411+
("col;select", "`col;select`"),
412+
("col\nname", "`col\nname`"),
413+
("test\\", "`test\\\\`"),
414+
("my_schema.my_table", "my_schema.my_table"),
415+
("my-schema.my-table", "`my-schema`.`my-table`"),
416+
("`my_table`", "`my_table`"),
417+
("`my_schema`.`my_table`", "`my_schema`.`my_table`"),
418+
("`table.with.dots`", "`table.with.dots`"),
419+
("`col\\`; DROP TABLE users; --`", "`col\\`; DROP TABLE users; --`"),
420+
(
421+
"`col\\\\`; DROP TABLE users; --`",
422+
"`\\`col\\\\\\\\\\`; DROP TABLE users; --\\``",
423+
),
406424
)
407425
for name, want in cases:
408426
with self.subTest(name=name):

0 commit comments

Comments
 (0)