Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
c1c23fb
Expose XML Expat 2.7.2 mitigation APIs
picnixz Sep 22, 2025
12bef9c
add tests
picnixz Sep 22, 2025
1d7e599
docs
picnixz Sep 22, 2025
3dcd9bd
NEWS
picnixz Sep 22, 2025
192fe08
Merge branch 'main' into feat/xml/mitigation-api-90949
picnixz Sep 22, 2025
0ecbd55
fix docs
picnixz Sep 22, 2025
07445ad
fix tests
picnixz Sep 22, 2025
9c7371f
regen SBOM
picnixz Sep 22, 2025
1085584
remove unused include
picnixz Sep 22, 2025
c10fe91
fix possible error handling
picnixz Sep 22, 2025
18d175f
undef macro after usage
picnixz Sep 22, 2025
911b2b7
Update Lib/test/test_pyexpat.py
picnixz Sep 22, 2025
d636685
update comments
picnixz Sep 22, 2025
b951065
use better test names
picnixz Sep 22, 2025
e11bf14
simplify roles usage
picnixz Sep 22, 2025
3e45613
prevent reparse deferral of Expat to blow up
picnixz Sep 22, 2025
fb83fb5
test better numeric values
picnixz Sep 22, 2025
7f91f2e
update docs
picnixz Sep 22, 2025
64af05c
avoid deprecated `XML_GetError{Line,Column}Number`
picnixz Sep 22, 2025
b01e53d
raise `NotImplementedError` for unavailable mitigation APIs
picnixz Sep 22, 2025
cd040bf
improve various wordings
picnixz Sep 23, 2025
a09cd15
avoid SBOM alteration
picnixz Sep 23, 2025
a3fc3b3
regen files
picnixz Sep 23, 2025
5afe1ad
improve test assertions
picnixz Sep 23, 2025
b6949dd
improve test genericity
picnixz Sep 23, 2025
bdbd382
split tests even if they could end up duplicated
picnixz Sep 23, 2025
0c03735
raise AttributeError when method is not available and reorder interface
picnixz Sep 23, 2025
220f3e2
reoder docs
picnixz Sep 23, 2025
9d538e4
address review
picnixz Sep 24, 2025
209f300
update capsule API
picnixz Sep 24, 2025
8407cfc
amend docs
picnixz Sep 24, 2025
e898811
use "NOTE:"
picnixz Sep 24, 2025
ce8cb48
address final comments
picnixz Sep 26, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Expose XML Expat 2.7.2 mitigation APIs
  • Loading branch information
picnixz committed Sep 22, 2025
commit c1c23fbf29411a1c52b326874a95df1b8da65d64
136 changes: 135 additions & 1 deletion Modules/clinic/pyexpat.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Modules/expat/expat_external.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

#ifndef Expat_External_INCLUDED
# define Expat_External_INCLUDED 1
/* Required so that functions in expat.h are declared */
#include "expat_config.h"
Comment thread
picnixz marked this conversation as resolved.
Outdated
/* Namespace external symbols to allow multiple libexpat version to
co-exist. */
#include "pyexpatns.h"
Expand Down
2 changes: 2 additions & 0 deletions Modules/expat/pyexpatns.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@
#define XmlPrologStateInit PyExpat_XmlPrologStateInit
#define XmlPrologStateInitExternalEntity PyExpat_XmlPrologStateInitExternalEntity
#define XML_ResumeParser PyExpat_XML_ResumeParser
#define XML_SetAllocTrackerActivationThreshold PyExpat_XML_SetAllocTrackerActivationThreshold
#define XML_SetAllocTrackerMaximumAmplification PyExpat_XML_SetAllocTrackerMaximumAmplification
#define XML_SetAttlistDeclHandler PyExpat_XML_SetAttlistDeclHandler
#define XML_SetBase PyExpat_XML_SetBase
#define XML_SetBillionLaughsAttackProtectionActivationThreshold PyExpat_XML_SetBillionLaughsAttackProtectionActivationThreshold
Expand Down
166 changes: 146 additions & 20 deletions Modules/pyexpat.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
#include "pycore_pyhash.h" // _Py_HashSecret
#include "pycore_traceback.h" // _PyTraceback_Add()

#include <float.h> // FLT_MAX
#include <stdbool.h>
#include <stddef.h> // offsetof()

#include "expat.h"
#include "pyexpat.h"

Expand Down Expand Up @@ -138,31 +140,72 @@ set_error_attr(PyObject *err, const char *name, int value)
return 1;
}

static PyObject *
format_xml_error(enum XML_Error code, int lineno, int column)
{
const char *errmsg = XML_ErrorString(code);
PyUnicodeWriter *writer = PyUnicodeWriter_Create(strlen(errmsg) + 1);
if (writer == NULL) {
return NULL;
}
if (PyUnicodeWriter_Format(writer,
"%s: line %i, column %i",
errmsg, lineno, column) < 0)
{
PyUnicodeWriter_Discard(writer);
return NULL;
}
return PyUnicodeWriter_Finish(writer);
}

static PyObject *
set_xml_error(pyexpat_state *state,
enum XML_Error code, int lineno, int column,
const char *errmsg)
{
PyObject *arg = errmsg == NULL
? format_xml_error(code, lineno, column)
: PyUnicode_FromStringAndSize(errmsg, strlen(errmsg));
if (arg == NULL) {
return NULL;
}
PyObject *res = PyObject_CallOneArg(state->error, arg);
Py_DECREF(arg);
if (
res != NULL
&& set_error_attr(res, "code", code)
&& set_error_attr(res, "lineno", lineno)
&& set_error_attr(res, "offset", column)
) {
PyErr_SetObject(state->error, res);
Py_DECREF(res);
}
return NULL;
}

#define SET_XML_ERROR(STATE, SELF, CODE, ERRMSG) \
do { \
XML_Parser parser = SELF->itself; \
assert(parser != NULL); \
int lineno = XML_GetErrorLineNumber(parser); \
int column = XML_GetErrorColumnNumber(parser); \
Comment thread
picnixz marked this conversation as resolved.
Outdated
(void)set_xml_error(state, CODE, lineno, column, ERRMSG); \
} while (0)

/* Build and set an Expat exception, including positioning
* information. Always returns NULL.
*/
static PyObject *
set_error(pyexpat_state *state, xmlparseobject *self, enum XML_Error code)
{
PyObject *err;
PyObject *buffer;
XML_Parser parser = self->itself;
int lineno = XML_GetErrorLineNumber(parser);
int column = XML_GetErrorColumnNumber(parser);
SET_XML_ERROR(state, self, code, NULL);
return NULL;
}

buffer = PyUnicode_FromFormat("%s: line %i, column %i",
XML_ErrorString(code), lineno, column);
if (buffer == NULL)
return NULL;
err = PyObject_CallOneArg(state->error, buffer);
Py_DECREF(buffer);
if ( err != NULL
&& set_error_attr(err, "code", code)
&& set_error_attr(err, "offset", column)
&& set_error_attr(err, "lineno", lineno)) {
PyErr_SetObject(state->error, err);
}
Py_XDECREF(err);
static PyObject *
set_invalid_arg(pyexpat_state *state, xmlparseobject *self, const char *errmsg)
{
SET_XML_ERROR(state, self, XML_ERROR_INVALID_ARGUMENT, errmsg);
return NULL;
}

Expand Down Expand Up @@ -1133,6 +1176,89 @@ pyexpat_xmlparser_UseForeignDTD_impl(xmlparseobject *self, PyTypeObject *cls,
}
#endif

#if XML_COMBINED_VERSION >= 20702

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that Expat >=2.7.2 is needed to offer this functionality, but maybe the the Python API should be available in any case and raise exceptions that the compile time Expat was not recent enough to support this feature. Is that what's happening? I remember we had this very question when introducing SetReparseDeferralEnabled.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argument Clinic will handle this automatically by "not" having the function (an AttributeError will be raised). For SetReparseDeferralEnabled, it appears that the API call will be a no-op otherwise.

Maybe it's better to raise NotImplementedError instead of the default AttributeError, as we usually do in ssl when the libssl backend is not the latest.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@picnixz I'd like to vote for NotImplementedError or no-op rather than AttributeError for now. I'll need to sleep over this and potentially dig up past communication on the reparse deferall thing. There was past rationale on this somewhere.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a NotImplementedError over a no-op, as otherwise people might assume that they are protected against some attacks while they are not.

@hartwork hartwork Sep 22, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@picnixz I should note that protection is active by default (or it would not have fixed the vulnerability). So a no-op in the tuning function would leave them protected under default settings, not unprotected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the #error approach. It simplifies the checks. I think I'll do a separate PR for that one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@picnixz please feel free to CC me for review there 👍

@picnixz picnixz Sep 23, 2025 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, if I include expat_config.h before, those symbols will anyway be defined (unless someone changes expat_config.h). But I don't think I need to assume that expat_config.h should be changed. Until now, it was not even used... (it wasn't included). I prefer not to overcomplicate the file (but at least now I know that it should be fine)

@hartwork hartwork Sep 23, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@picnixz file expat_config.h is auto-generated by the Expat build system (if systemwide Expat is used) and hardcoded by CPython when the bundle is used. So in the former case, XML_DTD and XML_GE could be defined or not: it's under the sysadmin's control.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh thanks for the explanation. I thought it was a python-only file.

/*[clinic input]
@permit_long_summary
@permit_long_docstring_body
pyexpat.xmlparser.SetAllocTrackerMaximumAmplification

cls: defining_class
max_factor: float
/

Sets the maximum amplification factor between direct input and bytes of dynamic memory allocated.

By default, parsers objects have a maximum amplification factor of 100.
Comment thread
picnixz marked this conversation as resolved.
Outdated

The amplification factor is calculated as "allocated / direct" while parsing,
where "direct" is the number of bytes read from the primary document in parsing
and "allocated" is the number of bytes of dynamic memory allocated in the parser
hierarchy.

The 'max_factor' value must be a non-NaN floating point value greater than
or equal to 1.0. Amplifications factors greater than 100 can been observed
Comment thread
picnixz marked this conversation as resolved.
Outdated
near the start of parsing even with benign files in practice. As such, the
upper bound must be carefully chosen so to avoid false positives.
Comment thread
picnixz marked this conversation as resolved.
Outdated
[clinic start generated code]*/

static PyObject *
pyexpat_xmlparser_SetAllocTrackerMaximumAmplification_impl(xmlparseobject *self,
PyTypeObject *cls,
float max_factor)
/*[clinic end generated code: output=6e44bd48c9b112a0 input=18e8d07329c0efda]*/
{
assert(self->itself != NULL);
if (XML_SetAllocTrackerMaximumAmplification(self->itself, max_factor) == XML_TRUE) {
Py_RETURN_NONE;
}
// XML_SetAllocTrackerMaximumAmplification() can fail if self->itself
// is not a root parser (currently, this is equivalent to be created
// by ExternalEntityParserCreate()) or if 'max_factor' is NaN or < 1.0.
//
// Expat does not provide a way to determine whether a parser is a root
// or not, nor does it provide a way to distinguish between failures in
// XML_SetAllocTrackerMaximumAmplification() (see gh-90949), we manually
// detect the factor out-of-range issue here so that users have a better
// error message.
pyexpat_state *state = PyType_GetModuleState(cls);
const char *message = (isnan(max_factor) || max_factor < 1.0f)
? "'max_factor' must be at least 1.0"
: "parser must be a root parser";
return set_invalid_arg(state, self, message);
}

/*[clinic input]
@permit_long_summary
@permit_long_docstring_body
pyexpat.xmlparser.SetAllocTrackerActivationThreshold

cls: defining_class
threshold: unsigned_long_long
/

Sets the number of allocated bytes of dynamic memory needed to activate protection against disproportionate use of RAM.

By default, parsers objects have an allocation activation threshold of 64 MiB.
Comment thread
picnixz marked this conversation as resolved.
Outdated
[clinic start generated code]*/

static PyObject *
pyexpat_xmlparser_SetAllocTrackerActivationThreshold_impl(xmlparseobject *self,
PyTypeObject *cls,
unsigned long long threshold)
/*[clinic end generated code: output=bed7e93207ba08c5 input=8453509a137a47c0]*/
{
assert(self->itself != NULL);
if (XML_SetAllocTrackerActivationThreshold(self->itself, threshold) == XML_TRUE) {
Py_RETURN_NONE;
}
// XML_SetAllocTrackerActivationThreshold() can only fail if self->itself
// is not a root parser (currently, this is equivalent to be created
// by ExternalEntityParserCreate()).
pyexpat_state *state = PyType_GetModuleState(cls);
return set_invalid_arg(state, self, "parser must be a root parser");
}
#endif

static struct PyMethodDef xmlparse_methods[] = {
PYEXPAT_XMLPARSER_PARSE_METHODDEF
PYEXPAT_XMLPARSER_PARSEFILE_METHODDEF
Expand All @@ -1141,9 +1267,9 @@ static struct PyMethodDef xmlparse_methods[] = {
PYEXPAT_XMLPARSER_GETINPUTCONTEXT_METHODDEF
PYEXPAT_XMLPARSER_EXTERNALENTITYPARSERCREATE_METHODDEF
PYEXPAT_XMLPARSER_SETPARAMENTITYPARSING_METHODDEF
#if XML_COMBINED_VERSION >= 19505
PYEXPAT_XMLPARSER_USEFOREIGNDTD_METHODDEF
#endif
PYEXPAT_XMLPARSER_SETALLOCTRACKERMAXIMUMAMPLIFICATION_METHODDEF
PYEXPAT_XMLPARSER_SETALLOCTRACKERACTIVATIONTHRESHOLD_METHODDEF
PYEXPAT_XMLPARSER_SETREPARSEDEFERRALENABLED_METHODDEF
PYEXPAT_XMLPARSER_GETREPARSEDEFERRALENABLED_METHODDEF
{NULL, NULL} /* sentinel */
Expand Down