Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7f5db21

Browse filesBrowse files
zzzeekGerrit Code Review
zzzeek
authored and
Gerrit Code Review
committed
Merge "audit all types for literal_execute() handling" into main
2 parents bafba7c + 4eba699 commit 7f5db21
Copy full SHA for 7f5db21

6 files changed

+193-1Lines changed: 193 additions & 1 deletion

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎doc/build/changelog/unreleased_20/13448.rst‎

Copy file name to clipboard
+17Lines changed: 17 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
.. change::
2+
:tags: bug, sql
3+
:tickets: 13448
4+
5+
Added auditing to the test suite which exercises the literal execute
6+
processors across all datatypes and dialects to ensure that string input is
7+
either appropriately rejected or correctly escaped. Literal execute
8+
processors are invoked when the :paramref:`.bindparam.literal_execute`
9+
parameter is used with an explicit :func:`.bindparam` object, which
10+
overrides DBAPI-native bind handling to render the value inline with the
11+
statement instead. Datatypes that were updated include the originally
12+
reported SQL Server ``Uuid`` / ``UNIQUEIDENTIFIER`` rendering which now
13+
escapes properly, the :class:`.JSONPATH` type that's currently
14+
PostgreSQL-only, and a full family of numeric types stemming from the
15+
:class:`_types.Float` and :class:`_types.Numeric` bases which now coerce
16+
the value to a number, rejecting non-numeric input. Thanks to Javid Khan
17+
for helping to identify the issue.
Collapse file

‎lib/sqlalchemy/dialects/mssql/base.py‎

Copy file name to clipboardExpand all lines: lib/sqlalchemy/dialects/mssql/base.py
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1561,7 +1561,7 @@ def literal_processor(self, dialect):
15611561
if self.native_uuid:
15621562

15631563
def process(value):
1564-
return f"""'{str(value).replace("''", "'")}'"""
1564+
return f"""'{str(value).replace("'", "''")}'"""
15651565

15661566
return process
15671567
else:
Collapse file

‎lib/sqlalchemy/dialects/postgresql/json.py‎

Copy file name to clipboardExpand all lines: lib/sqlalchemy/dialects/postgresql/json.py
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ def process(value: Any) -> Any:
5151
if isinstance(value, str):
5252
# If it's already a string assume that it's in json path
5353
# format. This allows using cast with json paths literals
54+
# Still need to process through super_proc for proper escaping
55+
if super_proc:
56+
value = super_proc(value)
5457
return value
5558
elif value:
5659
# If it's already a string assume that it's in json path
Collapse file

‎lib/sqlalchemy/sql/compiler.py‎

Copy file name to clipboardExpand all lines: lib/sqlalchemy/sql/compiler.py
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1872,6 +1872,13 @@ def _bind_processors(
18721872
),
18731873
)
18741874
for bindparam in self.bind_names
1875+
# literal_execute parameters are rendered into the SQL string
1876+
# via their literal_processor and never bound as values, so
1877+
# they do not need a bind processor. Skipping them also avoids
1878+
# invoking bind-processor construction that may require the
1879+
# DBAPI to be present (see asyncpg, psycopgcffi cases),
1880+
# facilitating testing.
1881+
if bindparam not in self.literal_execute_params
18751882
)
18761883
if value is not None
18771884
}
Collapse file

‎lib/sqlalchemy/sql/sqltypes.py‎

Copy file name to clipboardExpand all lines: lib/sqlalchemy/sql/sqltypes.py
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,14 @@ def get_dbapi_type(self, dbapi):
491491

492492
def literal_processor(self, dialect):
493493
def process(value):
494+
# the value is rendered into the SQL string directly and
495+
# unquoted; the database's native parsing does the numeric
496+
# conversion. don't convert to a Decimal or float here, as that
497+
# would alter the rendered representation (e.g. "1.0000" -> "1.0").
498+
# instead validate that the value parses as a number, so that a
499+
# non-numeric string can't be injected for a literal_execute
500+
# parameter, but render the original string form unchanged.
501+
decimal.Decimal(value)
494502
return str(value)
495503

496504
return process
Collapse file

‎test/sql/test_types.py‎

Copy file name to clipboardExpand all lines: test/sql/test_types.py
+157Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,43 @@ def _all_dialects():
118118
return [d.base.dialect() for d in _all_dialect_modules()]
119119

120120

121+
def _dialect_family(module_name):
122+
"""Return the dialect family (e.g. "mysql", "postgresql") for a module
123+
name, or None if the module is not part of a specific dialect.
124+
125+
"""
126+
prefix = "sqlalchemy.dialects."
127+
if module_name.startswith(prefix):
128+
return module_name[len(prefix) :].split(".")[0]
129+
return None
130+
131+
132+
def _all_dialect_impls():
133+
"""Yield all specific dialect implementation classes.
134+
135+
This includes each DBAPI-specific dialect like psycopg2, asyncpg,
136+
pymssql, cx_oracle, etc., not just the base dialects.
137+
"""
138+
seen = set()
139+
for dialect_mod in _all_dialect_modules():
140+
for attr in dir(dialect_mod):
141+
if attr.startswith("_") or attr == "base":
142+
continue
143+
try:
144+
submod = getattr(dialect_mod, attr)
145+
if (
146+
hasattr(submod, "__name__")
147+
and submod.__name__.startswith("sqlalchemy.dialects.")
148+
and hasattr(submod, "dialect")
149+
):
150+
dialect_cls = submod.dialect
151+
if dialect_cls not in seen:
152+
seen.add(dialect_cls)
153+
yield dialect_cls
154+
except Exception:
155+
pass
156+
157+
121158
def _types_for_mod(mod):
122159
for key in dir(mod):
123160
typ = getattr(mod, key)
@@ -159,6 +196,7 @@ def _all_types_w_their_dialect(omit_special_types=False):
159196
continue
160197
seen.add(typ)
161198
yield typ, default.DefaultDialect
199+
162200
for dialect in _all_dialect_modules():
163201
for typ in _types_for_mod(dialect):
164202
if typ in seen:
@@ -167,6 +205,63 @@ def _all_types_w_their_dialect(omit_special_types=False):
167205
yield typ, dialect.dialect
168206

169207

208+
def _all_types_w_all_dialect_impls(omit_special_types=False):
209+
"""Yield each type with every applicable dialect implementation.
210+
211+
Core types (defined in ``sqlalchemy.sql.sqltypes``) are paired with
212+
:class:`.DefaultDialect` plus every dialect implementation (asyncpg,
213+
psycopg2, pymssql, cx_oracle, etc.).
214+
215+
Dialect-specific types (e.g. ``mysql.SET``, ``postgresql.INET``) are only
216+
paired with the dialect implementations that belong to the same backend
217+
family (e.g. ``mysql.SET`` is not yielded with the PostgreSQL dialects).
218+
"""
219+
core_types = set()
220+
for typ in _types_for_mod(types):
221+
if omit_special_types and (
222+
typ
223+
in (
224+
TypeEngine,
225+
type_api.TypeEngineMixin,
226+
types.Variant,
227+
types.TypeDecorator,
228+
types.PickleType,
229+
)
230+
or type_api.TypeEngineMixin in typ.__bases__
231+
):
232+
continue
233+
234+
if typ in core_types:
235+
continue
236+
core_types.add(typ)
237+
yield typ, default.DefaultDialect
238+
239+
# Collect dialect-specific types grouped by their backend family. The
240+
# family is taken from the type's defining module so that a type only
241+
# travels with the dialects that actually implement it.
242+
types_by_family = {}
243+
for dialect_mod in _all_dialect_modules():
244+
for typ in _types_for_mod(dialect_mod):
245+
if typ in core_types:
246+
continue
247+
family = _dialect_family(typ.__module__)
248+
if family is None:
249+
# a non-dialect type re-exported from a dialect module;
250+
# treat it as a core type applicable to all dialects
251+
core_types.add(typ)
252+
else:
253+
types_by_family.setdefault(family, set()).add(typ)
254+
255+
# Yield each type with every specific dialect implementation, respecting
256+
# the family boundary for dialect-specific types.
257+
for dialect_cls in _all_dialect_impls():
258+
family = _dialect_family(dialect_cls.__module__)
259+
for typ in core_types:
260+
yield typ, dialect_cls
261+
for typ in types_by_family.get(family, ()):
262+
yield typ, dialect_cls
263+
264+
170265
def _get_instance(type_):
171266
if issubclass(type_, ARRAY):
172267
return type_(String)
@@ -412,6 +507,68 @@ def test_every_possible_type_can_be_decorated(self, typ, dialect_cls):
412507
is_true(issubclass(typ, impl._type_affinity))
413508

414509

510+
class LiteralExecuteTest(fixtures.TestBase):
511+
# base classes that carry a boolean variant flag which selects between
512+
# distinct literal_processor code paths (e.g. native vs. string
513+
# rendering); both variants of each such type must be tested.
514+
_variant_bases = [
515+
(sqltypes.Uuid, "as_uuid"),
516+
(sqltypes.NumericCommon, "asdecimal"),
517+
]
518+
519+
@testing.combinations(
520+
*[
521+
(t, d)
522+
for t, d in _all_types_w_all_dialect_impls(omit_special_types=True)
523+
]
524+
)
525+
def test_safestr_literal_execute(self, typ, dialect_cls):
526+
"""test for #13448"""
527+
528+
if issubclass(typ, ARRAY):
529+
insts = [typ(Integer)]
530+
elif issubclass(typ, pg.ENUM):
531+
insts = [typ(name="my_enum")]
532+
elif issubclass(typ, pg.DOMAIN):
533+
insts = [typ(name="my_domain", data_type=Integer)]
534+
elif issubclass(typ, mysql.SET):
535+
insts = [typ("a", "b", "c")]
536+
else:
537+
for base, flag in self._variant_bases:
538+
if issubclass(typ, base):
539+
insts = [typ(**{flag: True}), typ(**{flag: False})]
540+
break
541+
else:
542+
# instantiate plain. if this fails for a particular type,
543+
# *do not* add an except: clause here, add an instantiator
544+
# above for it. every type must be tested!
545+
insts = [typ()]
546+
547+
# note the payload deliberately avoids characters that a legitimate
548+
# processor may strip (e.g. the ``Uuid(as_uuid=False)`` renderer
549+
# removes ``-``); the single quote is the escape-critical character.
550+
value = "z' OR 1=1"
551+
552+
dialect = dialect_cls()
553+
554+
for inst in insts:
555+
stmt = bindparam("u", type_=inst, literal_execute=True)
556+
557+
try:
558+
result = stmt.compile(
559+
dialect=dialect
560+
)._process_parameters_for_postcompile({"u": value})
561+
except exc.CompileError:
562+
# it's good, invalid data was rejected
563+
continue
564+
else:
565+
# rendering a literal must never require the DBAPI to be
566+
# present; if a particular type/dialect can't do so, that is a
567+
# bug to fix rather than skip. every rendered literal must
568+
# escape the quote.
569+
assert "z'' OR 1=1" in result.statement
570+
571+
415572
class TypeAffinityTest(fixtures.TestBase):
416573
@testing.combinations(
417574
(String(), String),

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.