Skip to content

Commit e471446

Browse files
Escape single quotes in time-travel SQL string literals
Address review feedback on PR #4211 from @sfc-gh-yuwang: SQL string literals embedded in the time-travel clause (``statement``, ``stream``, ``version_tag``, and the string form of ``timestamp``) were interpolated raw — a tag name like ``release_'s`` produced broken SQL and an untrusted value could close the literal and inject arbitrary text (e.g. ``x'); DROP TABLE foo; --``). Centralize escaping in a single ``_quote`` helper inside ``generate_sql_clause`` that doubles embedded ``'`` to ``''`` (Snowflake's standard string-literal escape) and apply it to all four string-valued parameters. Numeric parameters (``offset``, ``version``) need no escaping. The pattern was pre-existing for ``statement``, ``stream``, and ``timestamp``; this commit hardens those at the same time so the four parameters share one consistent escape path. Added ``test_time_travel_string_literal_escaping`` to ``tests/unit/test_utils.py`` covering all four parameters with both benign embedded quotes (``release_'s``) and an explicit injection payload, plus the typed ``TO_TIMESTAMP_NTZ`` variant. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a766590 commit e471446

2 files changed

Lines changed: 80 additions & 5 deletions

File tree

src/snowflake/snowpark/_internal/utils.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2099,19 +2099,37 @@ def generate_sql_clause(self) -> str:
20992099
" AT (VERSION => 1234567890)" for Iceberg snapshot id time travel,
21002100
or " AT (VERSION_TAG => 'release_v1')" for Iceberg tag time
21012101
travel.
2102+
2103+
Note on escaping: string-valued parameters (``statement``,
2104+
``stream``, ``version_tag``, ``timestamp``) are embedded inside
2105+
single-quoted SQL literals. We double any embedded ``'`` to the
2106+
Snowflake-standard ``''`` escape sequence before interpolation
2107+
so a value like ``release_'s`` produces ``'release_''s'``
2108+
rather than breaking SQL or opening an injection surface
2109+
(e.g. ``'); DROP TABLE foo; --``). Numeric parameters
2110+
(``offset``, ``version``) are not quoted and don't need
2111+
escaping.
21022112
"""
2113+
2114+
def _quote(value: str) -> str:
2115+
# Snowflake's SQL string-literal escape is doubled single
2116+
# quotes; backslashes have no special meaning in
2117+
# single-quoted literals here, so we don't need to escape
2118+
# those.
2119+
return "'" + str(value).replace("'", "''") + "'"
2120+
21032121
clause = f" {self.time_travel_mode.upper()} "
21042122

21052123
if self.statement is not None:
2106-
clause += f"(STATEMENT => '{self.statement}')"
2124+
clause += f"(STATEMENT => {_quote(self.statement)})"
21072125
elif self.offset is not None:
21082126
clause += f"(OFFSET => {self.offset})"
21092127
elif self.stream is not None:
2110-
clause += f"(STREAM => '{self.stream}')"
2128+
clause += f"(STREAM => {_quote(self.stream)})"
21112129
elif self.version is not None:
21122130
clause += f"(VERSION => {self.version})"
21132131
elif self.version_tag is not None:
2114-
clause += f"(VERSION_TAG => '{self.version_tag}')"
2132+
clause += f"(VERSION_TAG => {_quote(self.version_tag)})"
21152133
elif self.timestamp is not None:
21162134
if self.timestamp_type is not None:
21172135
timestamp_type = self.timestamp_type.upper()
@@ -2123,9 +2141,9 @@ def generate_sql_clause(self) -> str:
21232141
func_name = "TO_TIMESTAMP_TZ"
21242142
else:
21252143
func_name = "TO_TIMESTAMP"
2126-
clause += f"(TIMESTAMP => {func_name}('{self.timestamp}'))"
2144+
clause += f"(TIMESTAMP => {func_name}({_quote(self.timestamp)}))"
21272145
else:
2128-
clause += f"(TIMESTAMP => '{self.timestamp}')"
2146+
clause += f"(TIMESTAMP => {_quote(self.timestamp)})"
21292147

21302148
return clause
21312149

tests/unit/test_utils.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,63 @@ def test_time_travel_version_tag():
10741074
TimeTravelConfig.validate_and_normalize_params(version_tag="release_v1")
10751075

10761076

1077+
def test_time_travel_string_literal_escaping():
1078+
"""SQL literals embedded in time-travel clauses (``statement``,
1079+
``stream``, ``version_tag``, string-form ``timestamp``) must
1080+
escape embedded single quotes by doubling them — Snowflake's
1081+
standard string-literal escape — so neither malicious nor
1082+
accidental ``'`` characters in the value can break the SQL
1083+
text or open an injection surface.
1084+
1085+
Pinned via this test rather than only at the call sites because
1086+
the four parameters share one code path (``_quote`` in
1087+
``generate_sql_clause``); a regression in any one of them would
1088+
fail here.
1089+
"""
1090+
# version_tag — the parameter introduced by this PR.
1091+
config = TimeTravelConfig.validate_and_normalize_params(
1092+
time_travel_mode="at", version_tag="release_'s"
1093+
)
1094+
assert config.generate_sql_clause() == " AT (VERSION_TAG => 'release_''s')"
1095+
1096+
# An attempted injection payload — the closing quote and the
1097+
# injected DROP must be fully neutralized by the doubled quotes.
1098+
config_injection = TimeTravelConfig.validate_and_normalize_params(
1099+
time_travel_mode="at", version_tag="x'); DROP TABLE foo; --"
1100+
)
1101+
assert (
1102+
config_injection.generate_sql_clause()
1103+
== " AT (VERSION_TAG => 'x''); DROP TABLE foo; --')"
1104+
)
1105+
1106+
# statement — same escape applies (pre-existing pattern hardened
1107+
# while we're here; raised in PR #4211 review).
1108+
stmt_config = TimeTravelConfig(time_travel_mode="AT", statement="01a' OR '1'='1")
1109+
assert (
1110+
stmt_config.generate_sql_clause() == " AT (STATEMENT => '01a'' OR ''1''=''1')"
1111+
)
1112+
1113+
# stream — same escape applies.
1114+
stream_config = TimeTravelConfig(time_travel_mode="AT", stream="my_stream'); --")
1115+
assert stream_config.generate_sql_clause() == " AT (STREAM => 'my_stream''); --')"
1116+
1117+
# timestamp (string form, no timestamp_type) — same escape.
1118+
ts_config = TimeTravelConfig(time_travel_mode="AT", timestamp="2024-01-01'); --")
1119+
assert ts_config.generate_sql_clause() == " AT (TIMESTAMP => '2024-01-01''); --')"
1120+
1121+
# timestamp (with typed TO_TIMESTAMP_*) — same escape inside the
1122+
# function-call argument.
1123+
ts_typed = TimeTravelConfig(
1124+
time_travel_mode="AT",
1125+
timestamp="2024-01-01'); --",
1126+
timestamp_type="NTZ",
1127+
)
1128+
assert (
1129+
ts_typed.generate_sql_clause()
1130+
== " AT (TIMESTAMP => TO_TIMESTAMP_NTZ('2024-01-01''); --'))"
1131+
)
1132+
1133+
10771134
def test_extract_time_travel_version_tag_option():
10781135
"""Test Iceberg tag option extraction for the reader API.
10791136

0 commit comments

Comments
 (0)