[SCM] PostGIS branch master updated. 3.7.0beta2-54-g290f43d8b
git at osgeo.org
git at osgeo.org
Sat Aug 15 11:31:06 PDT 2026
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "PostGIS".
The branch, master has been updated
via 290f43d8b0a8b038408daf5750c4400be9ab0e38 (commit)
via 2b586232514788f45e8b9b08dfa1ab8f64f04758 (commit)
from 3a11382f95bc2a44d46f65c3b1db93d8bce50ef0 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 290f43d8b0a8b038408daf5750c4400be9ab0e38
Merge: 3a11382f9 2b5862325
Author: Darafei Praliaskouski <komzpa at gmail.com>
Date: Sat Aug 15 11:31:05 2026 -0700
Merge pull request 'docs: drop DOCTYPE internal subset before docbook QA parse' (!747) from Komzpa/postgis:fix/docbook-entity-declaration-guard-20260815 into master
Pull 735 (commit 43e13fe22) replaced a blanket DOCTYPE rejection in
utils/docs/xml_tree.py with an expat pre-pass that collects declared
internal general entity names and a raw-byte regex that rejects the
document if the body references one of them. Two bypasses let an internal
general entity reach the real parse anyway: the regex only matches ASCII
entity names, so a name such as é referenced as &é; is never flagged, and
the pre-pass never expands parameter entities, so a general entity
declared indirectly through one, such as <!ENTITY % pe "<!ENTITY expand
'expanded'>"> followed by %pe;, is invisible to it while the real SAX
parser still expands it. Both are internal-entity-expansion issues, not
XXE: external entity fetching stays refused by the existing feature flags
and EntityResolver.
This replaces reference-scanning with removing the DOCTYPE's entire
internal subset before the real parse, located with expat's own
doctypedecl parsing rather than text search. With no entity declared, any
leftover &name; reference fails as an undefined entity on its own,
regardless of spelling, declaration path, or whether it sits in element
content or an attribute value (the latter is normalized before any
element handler runs, so it could not have been caught by scanning
content). postgis-out.xml's DOCTYPE, once regenerated, turns out to
declare a handful of literal entities in addition to SYSTEM ones, all
already substituted by xmllint --noent and hence dead by the time this
module sees the file, so dropping the subset is a no-op for our own
corpus; check-docbook-markup, exampletest-report, and ref-index all still
pass against it.
Added regression tests: a non-ASCII entity name reference, a
parameter-entity-injected general entity reference, and an
attribute-value entity reference are all rejected; the existing
harmless-DOCTYPE case (matching postgis-out.xml's shape) still parses.
References https://gitea.osgeo.org/postgis/postgis/pulls/735
Reviewed-on: https://gitea.osgeo.org/postgis/postgis/pulls/747
commit 2b586232514788f45e8b9b08dfa1ab8f64f04758
Author: Darafei Praliaskouski <me at komzpa.net>
Date: Sat Aug 15 21:28:46 2026 +0400
docs: drop DOCTYPE internal subset before docbook QA parse
Commit 43e13fe22 (pull 735) replaced a blanket DOCTYPE rejection with an
expat pre-pass that collects the names of internal (literal-value) general
entities a document declares, followed by a raw-byte regex search for
references to those names, rejecting the document if one is found. Two
gaps let attacker-controlled internal entities reach the real parse
anyway. First, the regex only matches ASCII entity names, so a document
that declares <!ENTITY é "..."> using a non-ASCII name and references it
as &é; is never flagged, and expat happily expands it during the real
parse. Second, the pre-pass only records entities declared directly with
<!ENTITY name "value">; it does not expand parameter entities, so a
general entity declared indirectly through a parameter entity's
replacement text, such as <!ENTITY % pe "<!ENTITY expand 'expanded'>">
followed by %pe;, is invisible to the pre-pass while the real SAX parser
expands parameter entities unconditionally and still declares and expands
the smuggled general entity. Both let internal entity expansion proceed
unchecked, which is an availability and content-injection risk through
entity-expansion amplification, not XXE: external entities were already
refused by the existing feature_external_ges/feature_external_pes
settings and the EntityResolver added in the same commit, and remain
refused here.
Trying to predict every reference to a dangerous entity with a pattern
match is inherently incomplete, so this replaces reference-scanning with
removing the DOCTYPE's entire internal subset before the real parse,
which is the only place a document can declare a general or parameter
entity with literal replacement text in the first place. With no entity
declared, expat rejects any leftover "&name;" reference as an undefined
entity on its own, a plain well-formedness error rather than a case this
module has to anticipate, no matter how the declaration or the reference
is spelled and regardless of whether the reference sits in element
content or an attribute value; the latter is substituted during
attribute-value normalization before any element handler runs and could
not have been caught by scanning content alone. The internal subset is
located with expat's own doctypedecl parsing, which is immune to the
false-positive class the original DOCTYPE-substring guard had (matching
the literal text "<!DOCTYPE" inside a CDATA-protected example) and
correctly treats quoted values and comments inside the subset as opaque,
unlike a hand-rolled bracket search.
The comment this commit replaces asserted that postgis-out.xml's DOCTYPE
declares only external SYSTEM entities; regenerating the file the way
doc/Makefile.in does shows that is not quite right, its internal subset
also declares a handful of literal entities such as last_release_version
and last_geos_release_version. xmllint --noent has already substituted
every reference to any of them by the time this module sees the file, so
both the SYSTEM and the literal declarations are dead text, and dropping
the whole internal subset is a no-op for our own corpus: doc/Makefile.in's
check-docbook-markup, exampletest-report, and ref-index targets against a
freshly regenerated postgis-out.xml all still succeed.
References https://gitea.osgeo.org/postgis/postgis/pulls/735
diff --git a/utils/docs/tests/test_docbook_qa.py b/utils/docs/tests/test_docbook_qa.py
index 2f4b07aa1..92965e76e 100644
--- a/utils/docs/tests/test_docbook_qa.py
+++ b/utils/docs/tests/test_docbook_qa.py
@@ -89,6 +89,55 @@ class DocBookSourceLintTest(unittest.TestCase):
tree = parse_xml(path)
self.assertEqual("book", tree.tree.getroot().tag.rsplit("}", 1)[-1])
+ def test_xml_tree_rejects_non_ascii_entity_reference(self):
+ # A regex that only matches ASCII entity names, [A-Za-z_][\\w.-]*,
+ # never sees "&é;": the guard must not depend on predicting
+ # every spelling a reference can take, so this must still be rejected.
+ path = write_tmp(
+ ".xml",
+ '<!DOCTYPE book [<!ENTITY é "expanded">]>'
+ + DOCBOOK_OPEN
+ + '<para>&é;</para>'
+ + DOCBOOK_CLOSE,
+ )
+
+ with self.assertRaises(Exception):
+ parse_xml(path)
+
+ def test_xml_tree_rejects_parameter_entity_injected_general_entity(self):
+ # The general entity "expand" is never declared directly; a parameter
+ # entity's replacement text declares it instead. A pre-pass that
+ # collects only directly-declared general entity names never sees it,
+ # so the guard must not rely on predicting every declaration path.
+ path = write_tmp(
+ ".xml",
+ "<!DOCTYPE book [\n"
+ "<!ENTITY % pe \"<!ENTITY expand 'expanded'>\">\n"
+ "%pe;\n"
+ "]>\n"
+ + DOCBOOK_OPEN
+ + '<para>&expand;</para>'
+ + DOCBOOK_CLOSE,
+ )
+
+ with self.assertRaises(Exception):
+ parse_xml(path)
+
+ def test_xml_tree_rejects_entity_reference_in_attribute_value(self):
+ # Attribute-value normalization substitutes entity references before
+ # any element/attribute handler ever sees them, so a reference cannot
+ # be caught by inspecting element content alone.
+ path = write_tmp(
+ ".xml",
+ '<!DOCTYPE book [<!ENTITY expand "expanded">]>'
+ + DOCBOOK_OPEN
+ + '<para role="&expand;">text</para>'
+ + DOCBOOK_CLOSE,
+ )
+
+ with self.assertRaises(Exception):
+ parse_xml(path)
+
def test_mixed_programlisting_dash_run_markers_and_sql_comments(self):
for output in ("left | right\n----|----\n1 | 2", "----RESULT output ---\n1"):
with self.subTest(output=output):
diff --git a/utils/docs/xml_tree.py b/utils/docs/xml_tree.py
index 502dcbecb..04a2c53ba 100644
--- a/utils/docs/xml_tree.py
+++ b/utils/docs/xml_tree.py
@@ -2,7 +2,7 @@
from __future__ import annotations
-import re
+import io
from dataclasses import dataclass
from pathlib import Path
import xml.etree.ElementTree as ET
@@ -78,44 +78,78 @@ class _TreeBuilder(ContentHandler):
current.text = (current.text or "") + content
-_GENERAL_ENTITY_REFERENCE_RE = re.compile(rb"&([A-Za-z_][\w.-]*);")
+def _internal_subset_bounds(data):
+ """Return the (start, end) byte offsets of the DOCTYPE's internal subset,
+ the "[" ... "]" span right after the doctype name, or None if the document
+ has no internal subset at all.
-
-def _internal_general_entity_names(path):
- """Return the names of literal-value ("internal") general entities the
- document's DOCTYPE declares, without resolving or fetching anything external.
-
- Our own generated postgis-out.xml carries a DOCTYPE whose internal subset
- declares dozens of SYSTEM entities; xmllint uses those only to assemble
- doc/*.xml into one file at build time, so by the time this module sees the
- output they are unreferenced text in the DOCTYPE. They are external, so
- ExternalEntityRefHandler is left unset here and they are never resolved or
- counted. Only an entity with a literal replacement value can inject
- attacker-controlled text or drive an entity-expansion blowup if the document
- goes on to reference it, so only those names are worth tracking.
+ A previous version of this guard located the DOCTYPE with a raw text
+ search for the bytes "<!DOCTYPE", which also matched lookalike text
+ protected inside a CDATA section (the X3D example in
+ reference_output.xml) and had no way to tell a literal "]" or "]>" inside
+ a quoted entity value or a DTD comment from the internal subset's real
+ closing bracket. expat parses the doctypedecl grammar production for
+ real: StartDoctypeDeclHandler only fires for a genuine prolog DOCTYPE, and
+ its CurrentByteIndex lands exactly on the internal subset's opening "[";
+ EndDoctypeDeclHandler's CurrentByteIndex lands exactly on the
+ declaration's closing ">", with any nested brackets, quoted strings, or
+ comments in between already correctly consumed by expat's own tokenizer.
+ Nothing here resolves or fetches anything external: no
+ ExternalEntityRefHandler is registered, and parameter-entity parsing is
+ left at its default (off), so a %-parameter-entity reference in the
+ internal subset is not expanded during this scan either. It does not need
+ to be: the whole internal subset is removed below regardless of what it
+ declares.
"""
- names = set()
+ bounds = []
- def entity_decl(name, is_parameter_entity, value, _base, _system_id, _public_id, _notation_name):
- if value is not None and not is_parameter_entity:
- names.add(name)
+ def start_doctype_decl(_name, _system_id, _public_id, has_internal_subset):
+ if has_internal_subset:
+ bounds.append(parser.CurrentByteIndex)
+
+ def end_doctype_decl():
+ if bounds:
+ bounds.append(parser.CurrentByteIndex)
parser = xml.parsers.expat.ParserCreate()
- parser.EntityDeclHandler = entity_decl
- with Path(path).open("rb") as handle:
- parser.ParseFile(handle)
- return names
+ parser.StartDoctypeDeclHandler = start_doctype_decl
+ parser.EndDoctypeDeclHandler = end_doctype_decl
+ parser.Parse(data, True)
+ return tuple(bounds) if len(bounds) == 2 else None
-def _reject_internal_entity_references(path):
- entity_names = _internal_general_entity_names(path)
- if not entity_names:
- return
+def _without_internal_subset(path):
+ """Return the document bytes with any DOCTYPE internal subset removed.
+
+ The internal subset is the only place a document can declare a general or
+ parameter entity with literal replacement text. Removing it outright,
+ rather than trying to predict which of its declared entities the document
+ body goes on to reference, is what makes this guard complete instead of
+ another partial pattern match: no entity is left declared for the real
+ parse below to expand, whether the reference appears in element content
+ or inside an attribute value (attribute-value normalization substitutes
+ entities before startElement handlers ever see them, so it cannot be
+ intercepted there), and no matter how the declaration or the reference is
+ spelled, including a non-ASCII entity name or a general entity declared
+ indirectly through a parameter entity. With no entity declared, expat
+ itself rejects any "&name;" reference left in the body as an undefined
+ entity, which is a plain well-formedness error, not a pattern this module
+ has to anticipate.
+
+ Our own generated postgis-out.xml carries a DOCTYPE whose internal subset
+ declares dozens of SYSTEM entities plus a handful of literal ones
+ (last_release_version and its neighbors): xmllint --noent has already
+ substituted every reference to any of them by the time this module sees
+ the file, so their declarations are dead text and dropping them is a
+ no-op. The same is true of any other DOCTYPE whose internal subset the
+ document does not actually depend on.
+ """
data = Path(path).read_bytes()
- for match in _GENERAL_ENTITY_REFERENCE_RE.finditer(data):
- name = match.group(1).decode("ascii", "replace")
- if name in entity_names:
- raise xml.sax.SAXException(f"entity references are not supported: &{name};")
+ bounds = _internal_subset_bounds(data)
+ if bounds is None:
+ return data
+ start, end = bounds
+ return data[:start] + data[end:]
class _EmptyEntityResolver(EntityResolver):
@@ -133,7 +167,6 @@ class _EmptyEntityResolver(EntityResolver):
def parse(path):
- _reject_internal_entity_references(path)
parser = xml.sax.make_parser()
parser.setFeature(feature_namespaces, True)
for feature in (feature_external_ges, feature_external_pes):
@@ -144,5 +177,8 @@ def parse(path):
parser.setEntityResolver(_EmptyEntityResolver())
builder = _TreeBuilder()
parser.setContentHandler(builder)
- parser.parse(str(Path(path)))
+ source = InputSource()
+ source.setSystemId(str(Path(path)))
+ source.setByteStream(io.BytesIO(_without_internal_subset(path)))
+ parser.parse(source)
return IndexedTree(ET.ElementTree(builder.root), builder.parents, builder.lines)
-----------------------------------------------------------------------
Summary of changes:
utils/docs/tests/test_docbook_qa.py | 49 +++++++++++++++++
utils/docs/xml_tree.py | 106 ++++++++++++++++++++++++------------
2 files changed, 120 insertions(+), 35 deletions(-)
hooks/post-receive
--
PostGIS
More information about the postgis-tickets
mailing list