Skip to content

Commit 0235fa6

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 0235fa6

2 files changed

Lines changed: 46 additions & 4 deletions

File tree

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

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,15 +384,45 @@ def ensure_where_clause(sql):
384384

385385
def escape_name(name):
386386
"""
387-
Apply backticks to the name that either contain '-' or
388-
' ', or is a Cloud Spanner's reserved keyword.
387+
Apply backticks to the name if it is not a valid regular ASCII identifier,
388+
or if it is a Cloud Spanner's reserved keyword.
389389
390390
:type name: str
391391
:param name: Name to escape.
392392
393393
:rtype: str
394394
:returns: Name escaped if it has to be escaped.
395395
"""
396-
if "-" in name or " " in name or name.upper() in SPANNER_RESERVED_KEYWORDS:
397-
return "`" + name + "`"
396+
if not name:
397+
return name
398+
399+
if "." in name:
400+
parts = name.split(".")
401+
return ".".join(escape_name(part) for part in parts)
402+
403+
if len(name) >= 2 and name.startswith("`") and name.endswith("`"):
404+
inner = name[1:-1]
405+
i = 0
406+
is_properly_quoted = True
407+
while i < len(inner):
408+
if inner[i] == "\\":
409+
i += 2
410+
elif inner[i] == "`":
411+
is_properly_quoted = False
412+
break
413+
else:
414+
i += 1
415+
if is_properly_quoted:
416+
return name
417+
418+
is_valid_regular_identifier = (
419+
(name[0].isalpha() or name[0] == "_")
420+
and all(c.isalnum() or c == "_" for c in name)
421+
and name.isascii()
422+
)
423+
424+
if not is_valid_regular_identifier or name.upper() in SPANNER_RESERVED_KEYWORDS:
425+
escaped = name.replace("\\", "\\\\").replace("`", "\\`")
426+
return f"`{escaped}`"
427+
398428
return name

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,18 @@ 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`"),
406418
)
407419
for name, want in cases:
408420
with self.subTest(name=name):

0 commit comments

Comments
 (0)