From: Mike Bayer Date: Wed, 22 Jul 2026 15:29:37 +0000 (-0400) Subject: audit all types for literal_execute() handling X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4eba6997dc0f4cd103d47bbc78a5aafaf0c137b1;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git audit all types for literal_execute() handling Added auditing to the test suite which exercises the literal execute processors across all datatypes and dialects to ensure that string input is either appropriately rejected or correctly escaped. Literal execute processors are invoked when the :paramref:`.bindparam.literal_execute` parameter is used with an explicit :func:`.bindparam` object, which overrides DBAPI-native bind handling to render the value inline with the statement instead. Datatypes that were updated include the originally reported SQL Server ``Uuid`` / ``UNIQUEIDENTIFIER`` rendering which now escapes properly, the :class:`.JSONPATH` type that's currently PostgreSQL-only, and a full family of numeric types stemming from the :class:`_types.Float` and :class:`_types.Numeric` bases which now coerce the value to a number, rejecting non-numeric input. Thanks to Javid Khan for helping to identify the issue. Fixes: #13448 Change-Id: Ic1a0643fa08e6ba92a54b6032355ef66f61fdb89 --- diff --git a/doc/build/changelog/unreleased_20/13448.rst b/doc/build/changelog/unreleased_20/13448.rst new file mode 100644 index 0000000000..f63b8e077b --- /dev/null +++ b/doc/build/changelog/unreleased_20/13448.rst @@ -0,0 +1,17 @@ +.. change:: + :tags: bug, sql + :tickets: 13448 + + Added auditing to the test suite which exercises the literal execute + processors across all datatypes and dialects to ensure that string input is + either appropriately rejected or correctly escaped. Literal execute + processors are invoked when the :paramref:`.bindparam.literal_execute` + parameter is used with an explicit :func:`.bindparam` object, which + overrides DBAPI-native bind handling to render the value inline with the + statement instead. Datatypes that were updated include the originally + reported SQL Server ``Uuid`` / ``UNIQUEIDENTIFIER`` rendering which now + escapes properly, the :class:`.JSONPATH` type that's currently + PostgreSQL-only, and a full family of numeric types stemming from the + :class:`_types.Float` and :class:`_types.Numeric` bases which now coerce + the value to a number, rejecting non-numeric input. Thanks to Javid Khan + for helping to identify the issue. diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index c9679f3007..3cdda50960 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -1561,7 +1561,7 @@ class MSUUid(sqltypes.Uuid): if self.native_uuid: def process(value): - return f"""'{str(value).replace("''", "'")}'""" + return f"""'{str(value).replace("'", "''")}'""" return process else: diff --git a/lib/sqlalchemy/dialects/postgresql/json.py b/lib/sqlalchemy/dialects/postgresql/json.py index 54b33fc65a..68f8d1844d 100644 --- a/lib/sqlalchemy/dialects/postgresql/json.py +++ b/lib/sqlalchemy/dialects/postgresql/json.py @@ -51,6 +51,9 @@ class JSONPathType(sqltypes.JSON.JSONPathType): if isinstance(value, str): # If it's already a string assume that it's in json path # format. This allows using cast with json paths literals + # Still need to process through super_proc for proper escaping + if super_proc: + value = super_proc(value) return value elif value: # If it's already a string assume that it's in json path diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 6462d1395b..19a7642d01 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -1872,6 +1872,13 @@ class SQLCompiler(Compiled): ), ) for bindparam in self.bind_names + # literal_execute parameters are rendered into the SQL string + # via their literal_processor and never bound as values, so + # they do not need a bind processor. Skipping them also avoids + # invoking bind-processor construction that may require the + # DBAPI to be present (see asyncpg, psycopgcffi cases), + # facilitating testing. + if bindparam not in self.literal_execute_params ) if value is not None } diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 7d10d19eb6..62b546765e 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -491,6 +491,14 @@ class NumericCommon(HasExpressionLookup, TypeEngineMixin, Generic[_N]): def literal_processor(self, dialect): def process(value): + # the value is rendered into the SQL string directly and + # unquoted; the database's native parsing does the numeric + # conversion. don't convert to a Decimal or float here, as that + # would alter the rendered representation (e.g. "1.0000" -> "1.0"). + # instead validate that the value parses as a number, so that a + # non-numeric string can't be injected for a literal_execute + # parameter, but render the original string form unchanged. + decimal.Decimal(value) return str(value) return process diff --git a/test/sql/test_types.py b/test/sql/test_types.py index fd82a34ea0..c0bde4e9bf 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -118,6 +118,43 @@ def _all_dialects(): return [d.base.dialect() for d in _all_dialect_modules()] +def _dialect_family(module_name): + """Return the dialect family (e.g. "mysql", "postgresql") for a module + name, or None if the module is not part of a specific dialect. + + """ + prefix = "sqlalchemy.dialects." + if module_name.startswith(prefix): + return module_name[len(prefix) :].split(".")[0] + return None + + +def _all_dialect_impls(): + """Yield all specific dialect implementation classes. + + This includes each DBAPI-specific dialect like psycopg2, asyncpg, + pymssql, cx_oracle, etc., not just the base dialects. + """ + seen = set() + for dialect_mod in _all_dialect_modules(): + for attr in dir(dialect_mod): + if attr.startswith("_") or attr == "base": + continue + try: + submod = getattr(dialect_mod, attr) + if ( + hasattr(submod, "__name__") + and submod.__name__.startswith("sqlalchemy.dialects.") + and hasattr(submod, "dialect") + ): + dialect_cls = submod.dialect + if dialect_cls not in seen: + seen.add(dialect_cls) + yield dialect_cls + except Exception: + pass + + def _types_for_mod(mod): for key in dir(mod): typ = getattr(mod, key) @@ -159,6 +196,7 @@ def _all_types_w_their_dialect(omit_special_types=False): continue seen.add(typ) yield typ, default.DefaultDialect + for dialect in _all_dialect_modules(): for typ in _types_for_mod(dialect): if typ in seen: @@ -167,6 +205,63 @@ def _all_types_w_their_dialect(omit_special_types=False): yield typ, dialect.dialect +def _all_types_w_all_dialect_impls(omit_special_types=False): + """Yield each type with every applicable dialect implementation. + + Core types (defined in ``sqlalchemy.sql.sqltypes``) are paired with + :class:`.DefaultDialect` plus every dialect implementation (asyncpg, + psycopg2, pymssql, cx_oracle, etc.). + + Dialect-specific types (e.g. ``mysql.SET``, ``postgresql.INET``) are only + paired with the dialect implementations that belong to the same backend + family (e.g. ``mysql.SET`` is not yielded with the PostgreSQL dialects). + """ + core_types = set() + for typ in _types_for_mod(types): + if omit_special_types and ( + typ + in ( + TypeEngine, + type_api.TypeEngineMixin, + types.Variant, + types.TypeDecorator, + types.PickleType, + ) + or type_api.TypeEngineMixin in typ.__bases__ + ): + continue + + if typ in core_types: + continue + core_types.add(typ) + yield typ, default.DefaultDialect + + # Collect dialect-specific types grouped by their backend family. The + # family is taken from the type's defining module so that a type only + # travels with the dialects that actually implement it. + types_by_family = {} + for dialect_mod in _all_dialect_modules(): + for typ in _types_for_mod(dialect_mod): + if typ in core_types: + continue + family = _dialect_family(typ.__module__) + if family is None: + # a non-dialect type re-exported from a dialect module; + # treat it as a core type applicable to all dialects + core_types.add(typ) + else: + types_by_family.setdefault(family, set()).add(typ) + + # Yield each type with every specific dialect implementation, respecting + # the family boundary for dialect-specific types. + for dialect_cls in _all_dialect_impls(): + family = _dialect_family(dialect_cls.__module__) + for typ in core_types: + yield typ, dialect_cls + for typ in types_by_family.get(family, ()): + yield typ, dialect_cls + + def _get_instance(type_): if issubclass(type_, ARRAY): return type_(String) @@ -412,6 +507,68 @@ class AdaptTest(fixtures.TestBase): is_true(issubclass(typ, impl._type_affinity)) +class LiteralExecuteTest(fixtures.TestBase): + # base classes that carry a boolean variant flag which selects between + # distinct literal_processor code paths (e.g. native vs. string + # rendering); both variants of each such type must be tested. + _variant_bases = [ + (sqltypes.Uuid, "as_uuid"), + (sqltypes.NumericCommon, "asdecimal"), + ] + + @testing.combinations( + *[ + (t, d) + for t, d in _all_types_w_all_dialect_impls(omit_special_types=True) + ] + ) + def test_safestr_literal_execute(self, typ, dialect_cls): + """test for #13448""" + + if issubclass(typ, ARRAY): + insts = [typ(Integer)] + elif issubclass(typ, pg.ENUM): + insts = [typ(name="my_enum")] + elif issubclass(typ, pg.DOMAIN): + insts = [typ(name="my_domain", data_type=Integer)] + elif issubclass(typ, mysql.SET): + insts = [typ("a", "b", "c")] + else: + for base, flag in self._variant_bases: + if issubclass(typ, base): + insts = [typ(**{flag: True}), typ(**{flag: False})] + break + else: + # instantiate plain. if this fails for a particular type, + # *do not* add an except: clause here, add an instantiator + # above for it. every type must be tested! + insts = [typ()] + + # note the payload deliberately avoids characters that a legitimate + # processor may strip (e.g. the ``Uuid(as_uuid=False)`` renderer + # removes ``-``); the single quote is the escape-critical character. + value = "z' OR 1=1" + + dialect = dialect_cls() + + for inst in insts: + stmt = bindparam("u", type_=inst, literal_execute=True) + + try: + result = stmt.compile( + dialect=dialect + )._process_parameters_for_postcompile({"u": value}) + except exc.CompileError: + # it's good, invalid data was rejected + continue + else: + # rendering a literal must never require the DBAPI to be + # present; if a particular type/dialect can't do so, that is a + # bug to fix rather than skip. every rendered literal must + # escape the quote. + assert "z'' OR 1=1" in result.statement + + class TypeAffinityTest(fixtures.TestBase): @testing.combinations( (String(), String),