mirror of
https://github.com/pgadmin-org/pgadmin4.git
synced 2026-08-17 16:34:44 -05:00
fix(sqli): HTML-escape description fields and harden qtLiteral
Fixes a SQL injection vulnerability where authenticated users could
break out of SQL string literals in COMMENT ON ... IS '<description>'
clauses by submitting an apostrophe-laden description through pgAdmin
dialogs. The original report covered domains; the patch expands the
fix to every site of the same pattern.
Three layers of defense:
1. Site fixes (16 places) — Replace '{{ x.description }}' with
{{ x.description|qtLiteral(conn) }} across templates for domains,
domain constraints, foreign tables, languages, event triggers,
and the views OID-lookup query. Plumbs conn=self.conn through
every render_template call that needed it. Also fixes a `{ % elif`
Jinja typo in foreign-table schema diff that was preventing the
elif branch from being reachable.
2. Driver hardening — qtLiteral (in utils/driver/psycopg3/__init__.py)
used to silently return the raw unescaped value when conn was
falsy. Now raises ValueError with a message pointing at the two
fixes (render_template(..., conn=) or pass conn as the second
argument). Surfaces this whole bug class loudly going forward,
and immediately uncovered 8 latent plumbing bugs in
schemas/__init__.py, schemas/functions/__init__.py,
schemas/tables/utils.py, foreign_servers/__init__.py, and 7 sites
in roles/__init__.py — all now fixed.
3. Regression tests (3 new files):
- test_comment_description_sql_escaping.py — renders each
previously-vulnerable template with an apostrophe-injection
payload and asserts the escaped fragment is present (15
scenarios).
- test_sql_string_literal_lint.py — walks every *.sql template,
flags every '{{ ... }}' single-quote-wrapped Jinja
interpolation, and compares against a curated allowlist (75
entries, each with a justification — OIDs, fixed enums,
server-derived identifiers, SQL-comment headers, etc.). New
occurrences fail the test until either qtLiteral is used or
an allowlist entry is added.
- test_qtliteral_requires_conn.py — unit test asserting the new
fail-fast behavior.
Reported by Jasser Chebbi (j3seer).
This commit is contained in:
@@ -673,7 +673,7 @@ class EventTriggerView(PGChildNodeView, SchemaDiffObjectCompare):
|
||||
data[arg] = old_data[arg]
|
||||
sql = render_template(
|
||||
"/".join([self.template_path, self._UPDATE_SQL]),
|
||||
data=data, o_data=old_data
|
||||
data=data, o_data=old_data, conn=self.conn
|
||||
)
|
||||
else:
|
||||
sql = self._get_create_with_grant_sql(data)
|
||||
|
||||
+1
-1
@@ -26,7 +26,7 @@ ALTER EVENT TRIGGER {{ conn|qtIdent(o_data.name) }}
|
||||
|
||||
{% if data.comment is defined and data.comment != o_data.comment %}
|
||||
COMMENT ON EVENT TRIGGER {{ conn|qtIdent(data.name) }}
|
||||
IS '{{ data.comment }}';
|
||||
IS {{ data.comment|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
|
||||
{% if data.enabled and data.enabled != o_data.enabled %}
|
||||
|
||||
+1
-1
@@ -821,7 +821,7 @@ class ForeignServerView(PGChildNodeView, SchemaDiffObjectCompare):
|
||||
is_valid_options = True
|
||||
|
||||
sql = render_template("/".join([self.template_path, self._ACL_SQL]),
|
||||
fsid=fsid)
|
||||
fsid=fsid, conn=self.conn)
|
||||
status, fs_rv_acl_res = self.conn.execute_dict(sql)
|
||||
if not status:
|
||||
return internal_server_error(errormsg=fs_rv_acl_res)
|
||||
|
||||
+1
-1
@@ -39,7 +39,7 @@ ALTER LANGUAGE {{ conn|qtIdent(data.name) }}
|
||||
{# ============= Update language comments ============= #}
|
||||
{% if data.description is defined and data.description != o_data.description %}
|
||||
COMMENT ON LANGUAGE {{ conn|qtIdent(data.name) }}
|
||||
IS '{{ data.description }}';
|
||||
IS {{ data.description|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
|
||||
|
||||
@@ -800,7 +800,8 @@ It may have been removed by another user.
|
||||
SQL = render_template(
|
||||
"/".join([self.template_path, 'sql/get_name.sql']),
|
||||
_=gettext,
|
||||
scid=scid
|
||||
scid=scid,
|
||||
conn=self.conn
|
||||
)
|
||||
|
||||
status, name = self.conn.execute_scalar(SQL)
|
||||
|
||||
@@ -776,7 +776,8 @@ AND relkind != 'c'))"""
|
||||
data.update(parse_sec_labels_from_db(data['seclabels']))
|
||||
|
||||
SQL = render_template("/".join([self.template_path,
|
||||
self._CREATE_SQL]), data=data)
|
||||
self._CREATE_SQL]),
|
||||
data=data, conn=self.conn)
|
||||
|
||||
sql_header = """-- DOMAIN: {0}.{1}\n\n""".format(
|
||||
data['basensp'], data['name'])
|
||||
@@ -838,7 +839,7 @@ AND relkind != 'c'))"""
|
||||
if 'fulltype' in data or 'basetype' in data or 'collname' in data:
|
||||
SQL = render_template(
|
||||
"/".join([self.template_path, 'domain_schema_diff.sql']),
|
||||
data=data, o_data=old_data)
|
||||
data=data, o_data=old_data, conn=self.conn)
|
||||
else:
|
||||
if is_schema_diff:
|
||||
data['is_schema_diff'] = True
|
||||
@@ -894,7 +895,7 @@ AND relkind != 'c'))"""
|
||||
else:
|
||||
SQL = render_template("/".join([self.template_path,
|
||||
self._CREATE_SQL]),
|
||||
data=data)
|
||||
data=data, conn=self.conn)
|
||||
return SQL.strip('\n'), data['name']
|
||||
|
||||
@check_precondition
|
||||
|
||||
+4
-2
@@ -620,7 +620,8 @@ class DomainConstraintView(PGChildNodeView):
|
||||
|
||||
SQL = render_template("/".join([self.template_path,
|
||||
self._CREATE_SQL]),
|
||||
data=data, domain=domain, schema=schema)
|
||||
data=data, domain=domain, schema=schema,
|
||||
conn=self.conn)
|
||||
|
||||
sql_header = """-- CHECK: {1}.{0}
|
||||
|
||||
@@ -696,7 +697,8 @@ class DomainConstraintView(PGChildNodeView):
|
||||
|
||||
SQL = render_template("/".join([self.template_path,
|
||||
self._CREATE_SQL]),
|
||||
data=data, domain=domain, schema=schema)
|
||||
data=data, domain=domain, schema=schema,
|
||||
conn=self.conn)
|
||||
if 'name' in data:
|
||||
return True, SQL.strip('\n'), data['name']
|
||||
else:
|
||||
|
||||
+1
-1
@@ -6,5 +6,5 @@ ALTER DOMAIN {{ conn|qtIdent(schema, domain) }}
|
||||
|
||||
|
||||
COMMENT ON CONSTRAINT {{ conn|qtIdent(data.name) }} ON DOMAIN {{ conn|qtIdent(schema, domain) }}
|
||||
IS '{{ data.description }}';{% endif %}
|
||||
IS {{ data.description|qtLiteral(conn) }};{% endif %}
|
||||
{% endif %}
|
||||
|
||||
+2
-2
@@ -20,14 +20,14 @@ ALTER DOMAIN {{ conn|qtIdent(data.basensp, data.name) }}
|
||||
{% if c.description %}
|
||||
|
||||
COMMENT ON CONSTRAINT {{ conn|qtIdent(c.conname) }} ON DOMAIN {{ conn|qtIdent(data.basensp, data.name) }}
|
||||
IS '{{ c.description }}';
|
||||
IS {{ c.description|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
{% endfor -%}
|
||||
{% endif %}
|
||||
|
||||
{% if data.description %}
|
||||
COMMENT ON DOMAIN {{ conn|qtIdent(data.basensp, data.name) }}
|
||||
IS '{{ data.description }}';{% endif -%}
|
||||
IS {{ data.description|qtLiteral(conn) }};{% endif -%}
|
||||
|
||||
{% if data.seclabels %}
|
||||
{% for r in data.seclabels %}
|
||||
|
||||
+3
-3
@@ -25,7 +25,7 @@ ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
{% if c.description %}
|
||||
|
||||
COMMENT ON CONSTRAINT {{ conn|qtIdent(c.conname) }} ON DOMAIN {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
IS '{{ c.description }}';
|
||||
IS {{ c.description|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
{% endfor -%}
|
||||
{% for c in data.constraints.changed %}{% if c.conname and c.consrc %}
|
||||
@@ -35,14 +35,14 @@ ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
{% if c.description %}
|
||||
|
||||
COMMENT ON CONSTRAINT {{ conn|qtIdent(c.conname) }} ON DOMAIN {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
IS '{{ c.description }}';
|
||||
IS {{ c.description|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
{% endfor -%}
|
||||
{% endif %}
|
||||
|
||||
{% if data.description %}
|
||||
COMMENT ON DOMAIN {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
IS '{{ data.description }}';{% endif -%}
|
||||
IS {{ data.description|qtLiteral(conn) }};{% endif -%}
|
||||
|
||||
{% if data.seclabels %}
|
||||
{% for r in data.seclabels %}
|
||||
|
||||
+1
-1
@@ -69,7 +69,7 @@ ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}
|
||||
{% if c.description %}
|
||||
|
||||
COMMENT ON CONSTRAINT {{ conn|qtIdent(c.conname) }} ON DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}
|
||||
IS '{{ c.description }}';
|
||||
IS {{ c.description|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
{% endfor -%}{% endif -%}
|
||||
{% set seclabels = data.seclabels %}
|
||||
|
||||
+1
-1
@@ -48,7 +48,7 @@ ALTER FOREIGN TABLE {{ conn|qtIdent(data.basensp, data.name) }}
|
||||
{% if data.description %}
|
||||
|
||||
COMMENT ON FOREIGN TABLE {{ conn|qtIdent(data.basensp, data.name) }}
|
||||
IS '{{ data.description }}';
|
||||
IS {{ data.description|qtLiteral(conn) }};
|
||||
{% endif -%}
|
||||
{% if data.columns and data.columns|length > 0 %}
|
||||
{% for c in data.columns %}
|
||||
|
||||
+3
-3
@@ -54,11 +54,11 @@ ALTER FOREIGN TABLE {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
{% if data.description %}
|
||||
|
||||
COMMENT ON FOREIGN TABLE {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
IS '{{ data.description }}';
|
||||
{ % elif o_data.description %}
|
||||
IS {{ data.description|qtLiteral(conn) }};
|
||||
{% elif o_data.description %}
|
||||
|
||||
COMMENT ON FOREIGN TABLE {{ conn|qtIdent(o_data.basensp, o_data.name) }}
|
||||
IS '{{ o_data.description }}';
|
||||
IS {{ o_data.description|qtLiteral(conn) }};
|
||||
{% endif -%}
|
||||
{% if acl %}
|
||||
|
||||
|
||||
@@ -1615,7 +1615,7 @@ class FunctionView(PGChildNodeView, DataTypeReader, SchemaDiffObjectCompare):
|
||||
render_template(
|
||||
'schemas/pg/#{0}#/sql/get_name.sql'.format(
|
||||
self.manager.version),
|
||||
scid=scid
|
||||
scid=scid, conn=self.conn
|
||||
)
|
||||
)
|
||||
if not status:
|
||||
|
||||
@@ -888,7 +888,7 @@ class BaseTableView(PGChildNodeView, BasePartitionTable, VacuumSettings):
|
||||
rules_sql += render_template("/".join(
|
||||
[self.rules_template_path, self._CREATE_SQL]),
|
||||
data=res_data, display_comments=display_comments,
|
||||
add_replace_clause=True
|
||||
add_replace_clause=True, conn=self.conn
|
||||
)
|
||||
|
||||
# Add into main sql
|
||||
|
||||
+1
-1
@@ -2,5 +2,5 @@
|
||||
{% if data %}
|
||||
SELECT c.oid, c.relname FROM pg_catalog.pg_class c
|
||||
LEFT OUTER JOIN pg_catalog.pg_namespace nsp on nsp.oid = c.relnamespace
|
||||
WHERE c.relname = {{ data.name |qtLiteral(conn) }} and nsp.nspname = '{{ data.schema }}';
|
||||
WHERE c.relname = {{ data.name |qtLiteral(conn) }} and nsp.nspname = {{ data.schema|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
|
||||
+1
-1
@@ -2,5 +2,5 @@
|
||||
{% if data %}
|
||||
SELECT c.oid, c.relname FROM pg_catalog.pg_class c
|
||||
LEFT OUTER JOIN pg_catalog.pg_namespace nsp on nsp.oid = c.relnamespace
|
||||
WHERE c.relname = {{ data.name|qtLiteral(conn) }} and nsp.nspname = '{{ data.schema}}';
|
||||
WHERE c.relname = {{ data.name|qtLiteral(conn) }} and nsp.nspname = {{ data.schema|qtLiteral(conn) }};
|
||||
{% endif %}
|
||||
|
||||
+358
@@ -0,0 +1,358 @@
|
||||
##########################################################################
|
||||
#
|
||||
# pgAdmin 4 - PostgreSQL Tools
|
||||
#
|
||||
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
|
||||
# This software is released under the PostgreSQL Licence
|
||||
#
|
||||
##########################################################################
|
||||
|
||||
"""Regression test: COMMENT ... IS '<description>' templates must escape the
|
||||
description through the qtLiteral filter so that user-supplied apostrophes
|
||||
cannot break out of the string literal.
|
||||
|
||||
This covers all template sites that previously interpolated descriptions as
|
||||
``'{{ value }}'`` and were vulnerable to SQL injection via the description
|
||||
input on the corresponding pgAdmin dialogs.
|
||||
"""
|
||||
|
||||
import os
|
||||
|
||||
from flask import Flask, render_template
|
||||
from jinja2 import FileSystemLoader
|
||||
|
||||
from pgadmin.utils.driver import get_driver
|
||||
from pgadmin.utils.route import BaseTestGenerator
|
||||
from config import PG_DEFAULT_DRIVER
|
||||
|
||||
|
||||
# A description containing an apostrophe and a trailing SQL statement. If the
|
||||
# template fails to escape, the rendered SQL closes the literal early and the
|
||||
# remaining text becomes executable SQL.
|
||||
PAYLOAD = "x'; DROP TABLE pg_class; --"
|
||||
|
||||
# After correct qtLiteral escaping the apostrophe is doubled and the value is
|
||||
# wrapped in single quotes.
|
||||
SAFE_FRAGMENT = "'x''; DROP TABLE pg_class; --'"
|
||||
|
||||
# What the vulnerable template used to render: the apostrophe is not doubled,
|
||||
# so the literal closes after ``x`` and the remainder becomes executable SQL.
|
||||
VULN_FRAGMENT = "'x'; DROP TABLE pg_class; --'"
|
||||
|
||||
|
||||
WEB_ROOT = os.path.realpath(
|
||||
os.path.join(os.path.dirname(os.path.realpath(__file__)),
|
||||
os.pardir, os.pardir, os.pardir, os.pardir, os.pardir,
|
||||
os.pardir)
|
||||
)
|
||||
|
||||
|
||||
class _FakeConn:
|
||||
"""psycopg.sql.Literal.as_string() accepts None as its context and still
|
||||
produces the canonical PostgreSQL-escaped literal. The production
|
||||
``qtLiteral`` short-circuits when its conn argument is falsy, so we hand
|
||||
it an object whose ``.conn`` is None: that exercises the real escape
|
||||
branch (``psycopg.sql.Literal(value).as_string(None)``) without needing a
|
||||
live PostgreSQL connection."""
|
||||
|
||||
conn = None
|
||||
|
||||
def __bool__(self):
|
||||
return True
|
||||
|
||||
|
||||
def _abs(*parts):
|
||||
return os.path.join(WEB_ROOT, *parts)
|
||||
|
||||
|
||||
# Jinja loader roots needed by the affected templates. Each template imports
|
||||
# macros via paths like ``macros/schemas/security.macros`` or
|
||||
# ``macros/privilege.macros`` which resolve against these roots.
|
||||
MACRO_ROOTS = [
|
||||
_abs('pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'templates'),
|
||||
_abs('pgadmin', 'browser', 'server_groups', 'servers', 'templates'),
|
||||
]
|
||||
|
||||
|
||||
class CommentDescriptionSQLEscapingTestCase(BaseTestGenerator):
|
||||
"""Verify each previously-vulnerable COMMENT template escapes the
|
||||
description through ``qtLiteral`` so apostrophes cannot terminate the
|
||||
SQL string literal early."""
|
||||
|
||||
scenarios = [
|
||||
(
|
||||
'Domain create - data.description (domain comment)',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'templates'),
|
||||
template='domains/sql/default/create.sql',
|
||||
ctx=dict(
|
||||
data=dict(name='d1', basensp='public', basetype='text',
|
||||
description=PAYLOAD),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Domain create - c.description (constraint comment)',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'templates'),
|
||||
template='domains/sql/default/create.sql',
|
||||
ctx=dict(
|
||||
data=dict(
|
||||
name='d1', basensp='public', basetype='text',
|
||||
constraints=[
|
||||
dict(conname='c1', consrc='VALUE > 0',
|
||||
description=PAYLOAD),
|
||||
],
|
||||
),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Domain update - c.description in constraints.added',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'templates'),
|
||||
template='domains/sql/default/update.sql',
|
||||
ctx=dict(
|
||||
data=dict(
|
||||
name='d1',
|
||||
constraints=dict(
|
||||
deleted=[], changed=[],
|
||||
added=[dict(conname='c1', consrc='VALUE > 0',
|
||||
description=PAYLOAD)],
|
||||
),
|
||||
seclabels=dict(),
|
||||
),
|
||||
o_data=dict(name='d1', basensp='public', constraints={}),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Domain schema diff - data.description (domain comment)',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'templates'),
|
||||
template='domains/sql/default/domain_schema_diff.sql',
|
||||
ctx=dict(
|
||||
data=dict(description=PAYLOAD),
|
||||
o_data=dict(name='d1', basensp='public', fulltype='text'),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Domain schema diff - c.description in constraints.added',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'templates'),
|
||||
template='domains/sql/default/domain_schema_diff.sql',
|
||||
ctx=dict(
|
||||
data=dict(
|
||||
constraints=dict(
|
||||
added=[dict(conname='c1', consrc='VALUE > 0',
|
||||
description=PAYLOAD)],
|
||||
changed=[],
|
||||
),
|
||||
),
|
||||
o_data=dict(name='d1', basensp='public', fulltype='text'),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Domain schema diff - c.description in constraints.changed',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'templates'),
|
||||
template='domains/sql/default/domain_schema_diff.sql',
|
||||
ctx=dict(
|
||||
data=dict(
|
||||
constraints=dict(
|
||||
added=[],
|
||||
changed=[dict(conname='c1', consrc='VALUE > 0',
|
||||
description=PAYLOAD)],
|
||||
),
|
||||
),
|
||||
o_data=dict(name='d1', basensp='public', fulltype='text'),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Domain constraint create - data.description',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'domains', 'domain_constraints',
|
||||
'templates'),
|
||||
template='domain_constraints/sql/default/create.sql',
|
||||
ctx=dict(
|
||||
data=dict(name='c1', consrc='VALUE > 0',
|
||||
description=PAYLOAD),
|
||||
schema='public', domain='d1',
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Foreign table create - data.description',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'foreign_tables', 'templates'),
|
||||
template='foreign_tables/sql/default/create.sql',
|
||||
ctx=dict(
|
||||
data=dict(
|
||||
name='ft1', basensp='public', ftsrvname='srv',
|
||||
description=PAYLOAD, columns=[], constraints=[],
|
||||
),
|
||||
is_sql=False,
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Foreign table schema diff - data.description',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'foreign_tables', 'templates'),
|
||||
template='foreign_tables/sql/default/'
|
||||
'foreign_table_schema_diff.sql',
|
||||
ctx=dict(
|
||||
data=dict(description=PAYLOAD, ftsrvname='srv'),
|
||||
o_data=dict(name='ft1', basensp='public',
|
||||
columns=[], constraints=[]),
|
||||
is_sql=False,
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Foreign table schema diff - o_data.description (elif branch)',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'foreign_tables', 'templates'),
|
||||
template='foreign_tables/sql/default/'
|
||||
'foreign_table_schema_diff.sql',
|
||||
ctx=dict(
|
||||
data=dict(ftsrvname='srv'),
|
||||
o_data=dict(name='ft1', basensp='public',
|
||||
columns=[], constraints=[],
|
||||
description=PAYLOAD),
|
||||
is_sql=False,
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Language update - data.description',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'languages', 'templates'),
|
||||
template='languages/sql/default/update.sql',
|
||||
ctx=dict(
|
||||
data=dict(name='plperl', description=PAYLOAD),
|
||||
o_data=dict(name='plperl', description='old',
|
||||
trusted=True, lanproc='h', laninl='i',
|
||||
lanval='v', lanowner='postgres'),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'Event trigger update - data.comment',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'event_triggers', 'templates'),
|
||||
template='event_triggers/sql/default/update.sql',
|
||||
ctx=dict(
|
||||
data=dict(name='et1', comment=PAYLOAD),
|
||||
o_data=dict(name='et1', comment='old',
|
||||
eventname='ddl_command_start',
|
||||
eventfunname='public.f', eventowner='postgres',
|
||||
enabled='O'),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'View OID lookup (pg) - data.schema',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'views', 'templates'),
|
||||
template='views/pg/default/sql/view_id.sql',
|
||||
ctx=dict(
|
||||
data=dict(name='v1', schema=PAYLOAD),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
(
|
||||
'View OID lookup (ppas) - data.schema',
|
||||
dict(
|
||||
template_root=_abs(
|
||||
'pgadmin', 'browser', 'server_groups', 'servers',
|
||||
'databases', 'schemas', 'views', 'templates'),
|
||||
template='views/ppas/default/sql/view_id.sql',
|
||||
ctx=dict(
|
||||
data=dict(name='v1', schema=PAYLOAD),
|
||||
conn=_FakeConn(),
|
||||
),
|
||||
),
|
||||
),
|
||||
]
|
||||
|
||||
def setUp(self):
|
||||
self.app_under_test = _FakeApp(self.template_root)
|
||||
|
||||
def runTest(self):
|
||||
with self.app_under_test.app_context():
|
||||
rendered = render_template(self.template, **self.ctx)
|
||||
|
||||
self.assertIn(
|
||||
SAFE_FRAGMENT, rendered,
|
||||
msg=(
|
||||
'Description was not safely escaped by qtLiteral.\n'
|
||||
'Expected fragment: {}\n'
|
||||
'Rendered output:\n{}'.format(SAFE_FRAGMENT, rendered)
|
||||
),
|
||||
)
|
||||
self.assertNotIn(
|
||||
VULN_FRAGMENT, rendered,
|
||||
msg=(
|
||||
'Rendered SQL still contains the unescaped (vulnerable) '
|
||||
'fragment.\nRendered output:\n{}'.format(rendered)
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
class _FakeApp(Flask):
|
||||
"""Minimal Flask app whose Jinja environment mirrors the production
|
||||
template filters (``qtLiteral``, ``qtIdent``, ``qtTypeIdent``) and whose
|
||||
loader can resolve both the entity's own templates and the macro files
|
||||
they import."""
|
||||
|
||||
def __init__(self, template_root):
|
||||
super().__init__('')
|
||||
driver = get_driver(PG_DEFAULT_DRIVER, self)
|
||||
self.jinja_env.filters['qtLiteral'] = driver.qtLiteral
|
||||
self.jinja_env.filters['qtIdent'] = driver.qtIdent
|
||||
self.jinja_env.filters['qtTypeIdent'] = driver.qtTypeIdent
|
||||
self.jinja_loader = FileSystemLoader([template_root] + MACRO_ROOTS)
|
||||
+511
@@ -0,0 +1,511 @@
|
||||
##########################################################################
|
||||
#
|
||||
# pgAdmin 4 - PostgreSQL Tools
|
||||
#
|
||||
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
|
||||
# This software is released under the PostgreSQL Licence
|
||||
#
|
||||
##########################################################################
|
||||
|
||||
"""Lint regression test: every ``'{{ ... }}'`` Jinja interpolation in a SQL
|
||||
template is a potential SQL injection sink — the unquoted-then-quoted form
|
||||
relies on the substituted value being free of apostrophes, which is almost
|
||||
never enforceable across the codebase.
|
||||
|
||||
The safe pattern is ``{{ value|qtLiteral(conn) }}`` (no surrounding quotes
|
||||
in the template — the filter wraps the value in PostgreSQL-escaped single
|
||||
quotes itself). See also ``test_comment_description_sql_escaping.py`` for
|
||||
the per-template behavioural test that exercises the COMMENT sites that
|
||||
motivated this lint.
|
||||
|
||||
This test walks every ``*.sql`` template under ``web/pgadmin/`` and compares
|
||||
the set of single-quote-wrapped Jinja interpolations against an explicit
|
||||
allowlist. Each allowlist entry carries a short justification.
|
||||
|
||||
If you see this test fail with a "new occurrence" message, you have two
|
||||
options:
|
||||
|
||||
1. Preferred: change ``'{{ x }}'`` to ``{{ x|qtLiteral(conn) }}`` in the
|
||||
template. Make sure the ``render_template`` caller passes ``conn=`` so
|
||||
the filter does not short-circuit.
|
||||
2. If the value is provably safe (e.g. a numeric OID looked up from
|
||||
pg_catalog, a hardcoded constant supplied by the handler, or a
|
||||
bounded enum validated server-side), add an entry to ``ALLOWLIST``
|
||||
below with a one-line reason.
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
from collections import Counter
|
||||
|
||||
from pgadmin.utils.route import BaseTestGenerator
|
||||
|
||||
|
||||
# Repository web/ directory.
|
||||
WEB_ROOT = os.path.realpath(
|
||||
os.path.join(os.path.dirname(os.path.realpath(__file__)),
|
||||
os.pardir, os.pardir, os.pardir, os.pardir, os.pardir,
|
||||
os.pardir)
|
||||
)
|
||||
|
||||
# Root inside web/ that the lint walks.
|
||||
SCAN_ROOT = os.path.join(WEB_ROOT, 'pgadmin')
|
||||
|
||||
# Single-quote-wrapped Jinja interpolation. The expression body cannot
|
||||
# contain ``}`` because ``}}`` ends the Jinja tag.
|
||||
PATTERN = re.compile(r"'\{\{[^}]+\}\}'")
|
||||
|
||||
|
||||
# Allowlist of currently-known occurrences. Each entry is
|
||||
# ``((relative_path, matched_fragment), count, reason)`` where:
|
||||
#
|
||||
# * ``relative_path`` is the SQL template path relative to ``web/``
|
||||
# (forward slashes on every platform).
|
||||
# * ``matched_fragment`` is the exact ``'{{ ... }}'`` slice as it appears
|
||||
# in the template (whitespace inside the braces matters).
|
||||
# * ``count`` is the number of times that fragment occurs in that file
|
||||
# (a single long Jinja line can carry multiple identical fragments).
|
||||
# * ``reason`` is a short justification: why this is not exploitable.
|
||||
#
|
||||
# Add new entries only when (1) the substituted value is genuinely safe by
|
||||
# construction, or (2) the impact is something other than SQL injection
|
||||
# against arbitrary data. Anything that takes free-form user input through
|
||||
# the request body MUST use ``qtLiteral`` instead of being allowlisted.
|
||||
|
||||
ALLOWLIST = [
|
||||
# ------------------------------------------------------------------
|
||||
# Server-internal numeric OIDs. ``oid``/``tid`` are integers looked up
|
||||
# from pg_catalog and rendered into a literal-shaped placeholder so
|
||||
# they can be compared against text columns / cast back via regclass.
|
||||
# Not user-supplied; cannot contain apostrophes.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/types/'
|
||||
'templates/types/pg/sql/default/get_subtypes.sql',
|
||||
"'{{ oid }}'"), 1,
|
||||
"OID is a server-internal integer from pg_catalog (proargtypes lookup)."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/types/'
|
||||
'templates/types/ppas/sql/default/get_subtypes.sql',
|
||||
"'{{ oid }}'"), 1,
|
||||
"OID is a server-internal integer from pg_catalog (proargtypes lookup)."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/publications/'
|
||||
'templates/publications/pg/default/sql/get_all_columns.sql',
|
||||
"'{{ tid }}'"), 1,
|
||||
"tid is a numeric table OID from the URL path, used with ::regclass."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/publications/'
|
||||
'templates/publications/ppas/default/sql/get_all_columns.sql',
|
||||
"'{{ tid }}'"), 1,
|
||||
"tid is a numeric table OID from the URL path, used with ::regclass."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Fixed enum strings supplied by the dashboard module. The value
|
||||
# passed for ``log_format`` is selected from the server's
|
||||
# ``log_destination`` GUC and can only be ``csvlog``, ``stderr`` or
|
||||
# ``jsonlog`` (the only values pg_current_logfile accepts).
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/dashboard/templates/dashboard/sql/default/logs.sql',
|
||||
"'{{log_format}}'"), 1,
|
||||
"log_format is a fixed enum (csvlog/stderr/jsonlog) from server GUC."),
|
||||
(('pgadmin/dashboard/templates/dashboard/sql/default/log_stat.sql',
|
||||
"'{{log_format}}'"), 1,
|
||||
"log_format is a fixed enum (csvlog/stderr/jsonlog) from server GUC."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# ``constraint_type`` is a single-char pg_constraint contype code
|
||||
# ('c', 'f', 'p', 'u', 'x', ...) passed by pgAdmin's own handler when
|
||||
# rendering one of these templates; never derived from request input.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/15_plus/properties.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/11_plus/properties.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/default/properties.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/default/get_oid.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/default/get_oid_with_transaction.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/default/get_name.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/index_constraint/sql/default/nodes.sql',
|
||||
"'{{constraint_type}}'"), 1,
|
||||
"Single-char pg_constraint.contype code, hardcoded by handler."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Index stat lookups for an already-existing table. ``schema`` and
|
||||
# ``table`` come from the browser tree node and ultimately from
|
||||
# pg_catalog. Cannot contain apostrophes for any object pgAdmin was
|
||||
# able to discover via its own browse queries (which use qtLiteral on
|
||||
# the way in), so this is not a SQL injection sink. A correctness
|
||||
# follow-up (legitimately named objects containing apostrophes) is
|
||||
# tracked separately.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/indexes/sql/default/coll_stats.sql',
|
||||
"'{{schema}}'"), 1,
|
||||
"Schema name from pg_catalog via browser tree, not request input."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/indexes/sql/default/coll_stats.sql',
|
||||
"'{{table}}'"), 1,
|
||||
"Table name from pg_catalog via browser tree, not request input."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/indexes/sql/16_plus/coll_stats.sql',
|
||||
"'{{schema}}'"), 1,
|
||||
"Schema name from pg_catalog via browser tree, not request input."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/tables/'
|
||||
'templates/indexes/sql/16_plus/coll_stats.sql',
|
||||
"'{{table}}'"), 1,
|
||||
"Table name from pg_catalog via browser tree, not request input."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Aggregate initial-condition strings. ``initial_val`` and
|
||||
# ``moving_initial_val`` come from the aggregate-creation form. The
|
||||
# session executing the SQL must already hold ``CREATE`` on the
|
||||
# target schema (CREATE AGGREGATE requirement), so the user can
|
||||
# already issue arbitrary SQL via Query Tool. Tracked as a
|
||||
# consistency follow-up; not a privilege-escalation sink.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/aggregates/'
|
||||
'templates/aggregates/sql/11_plus/create.sql',
|
||||
"'{{data.initial_val}}'"), 1,
|
||||
"CREATE AGGREGATE requires CREATE on schema; user already has SQL "
|
||||
"access. Tracked as consistency follow-up."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/aggregates/'
|
||||
'templates/aggregates/sql/11_plus/create.sql',
|
||||
"'{{data.moving_initial_val}}'"), 1,
|
||||
"CREATE AGGREGATE requires CREATE on schema; user already has SQL "
|
||||
"access. Tracked as consistency follow-up."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/aggregates/'
|
||||
'templates/aggregates/sql/12_plus/create.sql',
|
||||
"'{{data.initial_val}}'"), 1,
|
||||
"CREATE AGGREGATE requires CREATE on schema; user already has SQL "
|
||||
"access. Tracked as consistency follow-up."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/aggregates/'
|
||||
'templates/aggregates/sql/12_plus/create.sql',
|
||||
"'{{data.moving_initial_val}}'"), 1,
|
||||
"CREATE AGGREGATE requires CREATE on schema; user already has SQL "
|
||||
"access. Tracked as consistency follow-up."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/aggregates/'
|
||||
'templates/aggregates/sql/default/create.sql',
|
||||
"'{{data.initial_val}}'"), 1,
|
||||
"CREATE AGGREGATE requires CREATE on schema; user already has SQL "
|
||||
"access. Tracked as consistency follow-up."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/schemas/aggregates/'
|
||||
'templates/aggregates/sql/default/create.sql',
|
||||
"'{{data.moving_initial_val}}'"), 1,
|
||||
"CREATE AGGREGATE requires CREATE on schema; user already has SQL "
|
||||
"access. Tracked as consistency follow-up."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Subscription create/update parameters. ``sync`` (synchronous_commit
|
||||
# value), ``streaming`` (on/off/parallel), and ``origin`` (any/none)
|
||||
# are bounded enum values selected from form drop-downs; the
|
||||
# subscription form schema rejects free-form input. Subscription
|
||||
# management additionally requires the user to be a superuser
|
||||
# (CREATE SUBSCRIPTION). Tracked as a consistency follow-up.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/default/create.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/default/update.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/14_plus/create.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/14_plus/create.sql',
|
||||
"'{{ data.streaming}}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/14_plus/update.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/14_plus/update.sql',
|
||||
"'{{ data.streaming}}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/15_plus/create.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/15_plus/create.sql',
|
||||
"'{{ data.streaming}}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/15_plus/update.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/15_plus/update.sql',
|
||||
"'{{ data.streaming}}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/16_plus/create.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/16_plus/create.sql',
|
||||
"'{{ data.streaming}}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/16_plus/create.sql',
|
||||
"'{{ data.origin}}'"), 1,
|
||||
"Bounded enum (any/none); CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/16_plus/update.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/16_plus/update.sql',
|
||||
"'{{ data.streaming }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/16_plus/update.sql',
|
||||
"'{{ data.origin }}'"), 1,
|
||||
"Bounded enum (any/none); CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/17_plus/create.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/17_plus/create.sql',
|
||||
"'{{ data.streaming}}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/17_plus/create.sql',
|
||||
"'{{ data.origin}}'"), 1,
|
||||
"Bounded enum (any/none); CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/17_plus/update.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/17_plus/update.sql',
|
||||
"'{{ data.streaming }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/17_plus/update.sql',
|
||||
"'{{ data.origin }}'"), 1,
|
||||
"Bounded enum (any/none); CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/18_plus/update.sql',
|
||||
"'{{ data.sync }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/18_plus/update.sql',
|
||||
"'{{ data.streaming }}'"), 1,
|
||||
"Bounded enum from form schema; CREATE SUBSCRIPTION needs superuser."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/18_plus/update.sql',
|
||||
"'{{ data.origin }}'"), 1,
|
||||
"Bounded enum (any/none); CREATE SUBSCRIPTION needs superuser."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Subscription / publication name lookups. ``subname``, ``pubname``
|
||||
# and ``pname`` come from the browser tree (resolved from OID
|
||||
# references in pg_catalog), not from request body input.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/default/get_position.sql',
|
||||
"'{{ subname }}'"), 1,
|
||||
"Subscription name from pg_catalog via browser tree."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/subscriptions/'
|
||||
'templates/subscriptions/sql/default/dependencies.sql',
|
||||
"'{{subname}}'"), 1,
|
||||
"Subscription name from pg_catalog via browser tree."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/publications/'
|
||||
'templates/publications/pg/default/sql/get_position.sql',
|
||||
"'{{ pubname }}'"), 1,
|
||||
"Publication name from pg_catalog via browser tree."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/publications/'
|
||||
'templates/publications/pg/default/sql/dependencies.sql',
|
||||
"'{{ pname }}'"), 1,
|
||||
"Publication name from pg_catalog via browser tree."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/publications/'
|
||||
'templates/publications/ppas/default/sql/get_position.sql',
|
||||
"'{{ pubname }}'"), 1,
|
||||
"Publication name from pg_catalog via browser tree."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/publications/'
|
||||
'templates/publications/ppas/default/sql/dependencies.sql',
|
||||
"'{{ pname }}'"), 1,
|
||||
"Publication name from pg_catalog via browser tree."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# DBMS job scheduler ``CREATE`` templates emit lines that are entirely
|
||||
# inside SQL ``--`` line comments (display-only headers, never
|
||||
# executed by pgAdmin). Each scheduler entity has two such lines.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/'
|
||||
'dbms_jobs/templates/dbms_jobs/ppas/16_plus/create.sql',
|
||||
"'{{ job_name }}'"), 2,
|
||||
"Inside ``--`` SQL line comment (display header), not executed."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/'
|
||||
'dbms_schedules/templates/dbms_schedules/ppas/16_plus/create.sql',
|
||||
"'{{ schedule_name }}'"), 2,
|
||||
"Inside ``--`` SQL line comment (display header), not executed."),
|
||||
(('pgadmin/browser/server_groups/servers/databases/dbms_job_scheduler/'
|
||||
'dbms_programs/templates/dbms_programs/ppas/16_plus/create.sql',
|
||||
"'{{ program_name }}'"), 2,
|
||||
"Inside ``--`` SQL line comment (display header), not executed."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# grant_wizard SQL templates emit hardcoded display/typing constants
|
||||
# (object_type, icon, prokind, relkind, node_type) that the wizard's
|
||||
# own handler supplies as fixed string literals — not user input.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/default/sql/'
|
||||
'function.sql', "'{{ func_type }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/default/sql/'
|
||||
'function.sql', "'{{ icon }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/default/sql/'
|
||||
'view.sql', "'{{ ntype }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/default/sql/'
|
||||
'view.sql', "'{{ node_type }}'"), 1,
|
||||
"Hardcoded pg_class.relkind char supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/11_plus/sql/'
|
||||
'function.sql', "'{{ func_type }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/11_plus/sql/'
|
||||
'function.sql', "'{{ icon }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/pg/11_plus/sql/'
|
||||
'function.sql', "'{{ kind }}'"), 1,
|
||||
"Hardcoded pg_proc.prokind char supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/default/sql/'
|
||||
'function.sql', "'{{ func_type }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/default/sql/'
|
||||
'function.sql', "'{{ icon }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/default/sql/'
|
||||
'view.sql', "'{{ ntype }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/default/sql/'
|
||||
'view.sql', "'{{ view_icon }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/default/sql/'
|
||||
'view.sql', "'{{ node_type }}'"), 1,
|
||||
"Hardcoded pg_class.relkind char supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/11_plus/sql/'
|
||||
'function.sql', "'{{ func_type }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/11_plus/sql/'
|
||||
'function.sql', "'{{ icon }}'"), 1,
|
||||
"Hardcoded display constant supplied by grant_wizard handler."),
|
||||
(('pgadmin/tools/grant_wizard/templates/grant_wizard/ppas/11_plus/sql/'
|
||||
'function.sql', "'{{ kind }}'"), 1,
|
||||
"Hardcoded pg_proc.prokind char supplied by grant_wizard handler."),
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# search_objects: ``obj_type`` is one of the fixed object-kind strings
|
||||
# baked into the search handler's own dispatch table.
|
||||
# ------------------------------------------------------------------
|
||||
(('pgadmin/tools/search_objects/templates/search_objects/sql/ppas/'
|
||||
'11_plus/search.sql', "'{{ obj_type }}'"), 1,
|
||||
"Fixed object-kind enum supplied by search_objects handler."),
|
||||
(('pgadmin/tools/search_objects/templates/search_objects/sql/ppas/'
|
||||
'12_plus/search.sql', "'{{ obj_type }}'"), 1,
|
||||
"Fixed object-kind enum supplied by search_objects handler."),
|
||||
(('pgadmin/tools/search_objects/templates/search_objects/sql/ppas/'
|
||||
'default/search.sql', "'{{ obj_type }}'"), 1,
|
||||
"Fixed object-kind enum supplied by search_objects handler."),
|
||||
]
|
||||
|
||||
|
||||
def _scan_sql_templates():
|
||||
"""Return Counter[(rel_path, fragment)] of every ``'{{ ... }}'``
|
||||
occurrence under SCAN_ROOT."""
|
||||
counter = Counter()
|
||||
for dirpath, _dirs, files in os.walk(SCAN_ROOT):
|
||||
for fname in files:
|
||||
if not fname.endswith('.sql'):
|
||||
continue
|
||||
full = os.path.join(dirpath, fname)
|
||||
rel = os.path.relpath(full, WEB_ROOT).replace(os.sep, '/')
|
||||
with open(full, encoding='utf-8') as fh:
|
||||
for line in fh:
|
||||
for m in PATTERN.finditer(line):
|
||||
counter[(rel, m.group(0))] += 1
|
||||
return counter
|
||||
|
||||
|
||||
def _allowlist_counter():
|
||||
counter = Counter()
|
||||
for key, count, _reason in ALLOWLIST:
|
||||
counter[key] += count
|
||||
return counter
|
||||
|
||||
|
||||
def _allowlist_reasons():
|
||||
return {key: reason for key, _count, reason in ALLOWLIST}
|
||||
|
||||
|
||||
class SQLStringLiteralInterpolationLintTestCase(BaseTestGenerator):
|
||||
"""Fail if any new ``'{{ ... }}'`` single-quote-wrapped Jinja
|
||||
interpolation appears in a SQL template without being either replaced
|
||||
with ``qtLiteral`` or added to ``ALLOWLIST`` above."""
|
||||
|
||||
scenarios = [('Lint *.sql templates for unescaped Jinja literals',
|
||||
dict())]
|
||||
|
||||
def runTest(self):
|
||||
actual = _scan_sql_templates()
|
||||
expected = _allowlist_counter()
|
||||
reasons = _allowlist_reasons()
|
||||
|
||||
new = [] # found in code, not allowlisted (or count too high)
|
||||
stale = [] # allowlisted, not found in code (or count too low)
|
||||
|
||||
for key, n in actual.items():
|
||||
allowed = expected.get(key, 0)
|
||||
if n > allowed:
|
||||
new.append((key, n - allowed))
|
||||
|
||||
for key, n in expected.items():
|
||||
present = actual.get(key, 0)
|
||||
if n > present:
|
||||
stale.append((key, n - present, reasons[key]))
|
||||
|
||||
messages = []
|
||||
if new:
|
||||
lines = [
|
||||
('FOUND unallowed single-quote-wrapped Jinja '
|
||||
'interpolations. Replace with `{{ x|qtLiteral(conn) }}` '
|
||||
'(no surrounding quotes) — and pass conn= to '
|
||||
'render_template — or add an explicit ALLOWLIST entry '
|
||||
'in test_sql_string_literal_lint.py with a reason.\n')
|
||||
]
|
||||
for (rel, frag), n in sorted(new):
|
||||
lines.append(' + {} :: {} (x{})'.format(rel, frag, n))
|
||||
messages.append('\n'.join(lines))
|
||||
if stale:
|
||||
lines = [
|
||||
('STALE ALLOWLIST entries (no longer present in code). '
|
||||
'Remove them from test_sql_string_literal_lint.py.\n')
|
||||
]
|
||||
for (rel, frag), n, reason in sorted(stale):
|
||||
lines.append(' - {} :: {} (x{}) // {}'
|
||||
.format(rel, frag, n, reason))
|
||||
messages.append('\n'.join(lines))
|
||||
if messages:
|
||||
self.fail('\n\n'.join(messages))
|
||||
@@ -733,7 +733,8 @@ rolmembership:{
|
||||
def list(self, gid, sid):
|
||||
status, res = self.conn.execute_dict(
|
||||
render_template(
|
||||
self.sql_path + self._PROPERTIES_SQL
|
||||
self.sql_path + self._PROPERTIES_SQL,
|
||||
conn=self.conn
|
||||
)
|
||||
)
|
||||
|
||||
@@ -752,7 +753,8 @@ rolmembership:{
|
||||
def nodes(self, gid, sid):
|
||||
|
||||
status, rset = self.conn.execute_2darray(
|
||||
render_template(self.sql_path + self._NODES_SQL)
|
||||
render_template(self.sql_path + self._NODES_SQL,
|
||||
conn=self.conn)
|
||||
)
|
||||
|
||||
if not status:
|
||||
@@ -783,7 +785,7 @@ rolmembership:{
|
||||
status, rset = self.conn.execute_2darray(
|
||||
render_template(
|
||||
self.sql_path + self._NODES_SQL,
|
||||
rid=rid
|
||||
rid=rid, conn=self.conn
|
||||
)
|
||||
)
|
||||
|
||||
@@ -877,7 +879,7 @@ rolmembership:{
|
||||
status, res = self.conn.execute_dict(
|
||||
render_template(
|
||||
self.sql_path + self._PROPERTIES_SQL,
|
||||
rid=rid
|
||||
rid=rid, conn=self.conn
|
||||
)
|
||||
)
|
||||
|
||||
@@ -998,7 +1000,7 @@ rolmembership:{
|
||||
|
||||
status, rset = self.conn.execute_dict(
|
||||
render_template(self.sql_path + self._NODES_SQL,
|
||||
rid=rid
|
||||
rid=rid, conn=self.conn
|
||||
)
|
||||
)
|
||||
|
||||
@@ -1042,7 +1044,7 @@ rolmembership:{
|
||||
|
||||
status, rset = self.conn.execute_dict(
|
||||
render_template(self.sql_path + self._NODES_SQL,
|
||||
rid=rid
|
||||
rid=rid, conn=self.conn
|
||||
)
|
||||
)
|
||||
|
||||
@@ -1280,7 +1282,7 @@ rolmembership:{
|
||||
|
||||
status, rset = self.conn.execute_dict(
|
||||
render_template(self.sql_path + 'variables.sql',
|
||||
rid=rid
|
||||
rid=rid, conn=self.conn
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@@ -309,16 +309,23 @@ class Driver(BaseDriver):
|
||||
|
||||
@staticmethod
|
||||
def qtLiteral(value, conn, force_quote=False):
|
||||
res = value
|
||||
if not conn:
|
||||
raise ValueError(
|
||||
"qtLiteral requires a connection: without one, escaping "
|
||||
"silently degrades to returning the raw value, which is a "
|
||||
"SQL injection sink. When using the Jinja filter, ensure "
|
||||
"render_template is called with conn=<conn>; when calling "
|
||||
"from Python, pass the connection as the second argument."
|
||||
)
|
||||
|
||||
if conn:
|
||||
try:
|
||||
if not isinstance(conn, psycopg.Connection) and \
|
||||
not isinstance(conn, psycopg.AsyncConnection):
|
||||
conn = conn.conn
|
||||
res = psycopg.sql.Literal(value).as_string(conn).strip()
|
||||
except Exception:
|
||||
print("Exception", value)
|
||||
res = value
|
||||
try:
|
||||
if not isinstance(conn, psycopg.Connection) and \
|
||||
not isinstance(conn, psycopg.AsyncConnection):
|
||||
conn = conn.conn
|
||||
res = psycopg.sql.Literal(value).as_string(conn).strip()
|
||||
except Exception:
|
||||
print("Exception", value)
|
||||
|
||||
if force_quote is True:
|
||||
# Convert the input to the string to use the startsWith(...)
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
##########################################################################
|
||||
#
|
||||
# pgAdmin 4 - PostgreSQL Tools
|
||||
#
|
||||
# Copyright (C) 2013 - 2026, The pgAdmin Development Team
|
||||
# This software is released under the PostgreSQL Licence
|
||||
#
|
||||
##########################################################################
|
||||
|
||||
"""Unit test: ``qtLiteral`` must refuse to escape without a connection.
|
||||
|
||||
Historically, ``qtLiteral`` silently short-circuited when its ``conn``
|
||||
argument was falsy and returned the raw value unchanged. Combined with
|
||||
templates that wrapped the result in single quotes (``'{{ x|qtLiteral }}'``
|
||||
or callers that forgot to pass ``conn=`` to ``render_template``), this
|
||||
produced unescaped SQL string literals that allowed apostrophe-based
|
||||
injection. The driver now raises so the failure is loud."""
|
||||
|
||||
from pgadmin.utils.driver import get_driver
|
||||
from pgadmin.utils.route import BaseTestGenerator
|
||||
from config import PG_DEFAULT_DRIVER
|
||||
|
||||
|
||||
class QtLiteralRequiresConnTestCase(BaseTestGenerator):
|
||||
|
||||
scenarios = [('qtLiteral raises ValueError when conn is falsy', dict())]
|
||||
|
||||
def runTest(self):
|
||||
driver = get_driver(PG_DEFAULT_DRIVER)
|
||||
for bad in (None, 0, '', False):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
driver.qtLiteral("any value", bad)
|
||||
self.assertIn('qtLiteral requires a connection',
|
||||
str(ctx.exception))
|
||||
Reference in New Issue
Block a user