Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 2 additions & 2 deletions Doc/library/pyexpat.rst
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ common XML vulnerabilities.
.. note::

The maximum amplification factor is only considered if the threshold
that can be adjusted :meth:`.SetAllocTrackerActivationThreshold` is
exceeded.
that can be adjusted by :meth:`.SetAllocTrackerActivationThreshold`
is exceeded.

.. versionadded:: next

Expand Down
4 changes: 2 additions & 2 deletions Doc/whatsnew/3.15.rst
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,8 @@ unittest
xml.parsers.expat
-----------------

* Add :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerActivationThreshold`
and :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerMaximumAmplification`
* Add :meth:`~xml.parsers.expat.xmlparser.SetAllocTrackerActivationThreshold`
and :meth:`~xml.parsers.expat.xmlparser.SetAllocTrackerMaximumAmplification`
to :ref:`xmlparser <xmlparser-objects>` objects to prevent use of
disproportional amounts of dynamic memory from within an Expat parser.
(Contributed by Bénédikt Tran in :gh:`90949`.)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Add :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerActivationThreshold`
and :func:`~xml.parsers.expat.xmlparser.SetAllocTrackerMaximumAmplification`
Add :meth:`~xml.parsers.expat.xmlparser.SetAllocTrackerActivationThreshold`
and :meth:`~xml.parsers.expat.xmlparser.SetAllocTrackerMaximumAmplification`
to :ref:`xmlparser <xmlparser-objects>` objects to prevent use of
disproportional amounts of dynamic memory from within an Expat parser.
Patch by Bénédikt Tran.
121 changes: 68 additions & 53 deletions Modules/pyexpat.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ CALL_XML_HANDLER_SETTER(const struct HandlerInfo *handler_info,
}

static int
set_error_code(PyObject *err, enum XML_Error code)
set_xml_error_attr_code(PyObject *err, enum XML_Error code)
{
PyObject *v = PyLong_FromLong((long)code);
int ok = v != NULL && PyObject_SetAttr(err, &_Py_ID(code), v) != -1;
Expand All @@ -137,7 +137,7 @@ set_error_code(PyObject *err, enum XML_Error code)
* false on an exception.
*/
static int
set_error_location(PyObject *err, const char *name, XML_Size value)
set_xml_error_attr_location(PyObject *err, const char *name, XML_Size value)
{
PyObject *v = PyLong_FromSize_t((size_t)value);
int ok = v != NULL && PyObject_SetAttrString(err, name, v) != -1;
Expand All @@ -146,42 +146,32 @@ set_error_location(PyObject *err, const char *name, XML_Size value)
}


static PyObject *
format_xml_error(enum XML_Error code, XML_Size lineno, XML_Size 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 %zu, column %zu",
errmsg, (size_t)lineno, (size_t)column) < 0)
{
PyUnicodeWriter_Discard(writer);
return NULL;
}
return PyUnicodeWriter_Finish(writer);
}

static PyObject *
set_xml_error(pyexpat_state *state,
enum XML_Error code, XML_Size lineno, XML_Size column,
const char *errmsg)
{
PyObject *arg = errmsg == NULL
? format_xml_error(code, lineno, column)
: PyUnicode_FromStringAndSize(errmsg, strlen(errmsg));
PyObject *arg;
if (errmsg == NULL) {
arg = PyUnicode_FromFormat(
"%s: line %zu, column %zu",
XML_ErrorString(code),
(size_t)lineno, (size_t)column
);
}
else {
arg = 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_code(res, code)
&& set_error_location(res, "lineno", lineno)
&& set_error_location(res, "offset", column)
&& set_xml_error_attr_code(res, code)
&& set_xml_error_attr_location(res, "lineno", lineno)
&& set_xml_error_attr_location(res, "offset", column)
) {
PyErr_SetObject(state->error, res);
}
Expand Down Expand Up @@ -1184,6 +1174,50 @@ pyexpat_xmlparser_UseForeignDTD_impl(xmlparseobject *self, PyTypeObject *cls,
}
#endif

#if XML_COMBINED_VERSION >= 20702
static PyObject *
set_activation_threshold(xmlparseobject *self,
PyTypeObject *cls,
unsigned long long threshold,
XML_Bool (*setter)(XML_Parser, unsigned long long))
{
assert(self->itself != NULL);
if (setter(self->itself, threshold) == XML_TRUE) {
Py_RETURN_NONE;
}
// The setter fails if self->itself is NULL (which is not possible here)
// or is a non-root parser, which currently only happens for parsers
// created by ExternalEntityParserCreate().
pyexpat_state *state = PyType_GetModuleState(cls);
return set_invalid_arg(state, self, "parser must be a root parser");
}

static PyObject *
set_maximum_amplification(xmlparseobject *self,
PyTypeObject *cls,
float max_factor,
XML_Bool (*setter)(XML_Parser, float))
{
assert(self->itself != NULL);
if (setter(self->itself, max_factor) == XML_TRUE) {
Py_RETURN_NONE;
}
// The setter fails if self->itself is NULL (which is not possible here),
// is a non-root parser, which currently only happens for parsers created
// by ExternalEntityParserCreate(), or if 'max_factor' is NaN or < 1.0.
pyexpat_state *state = PyType_GetModuleState(cls);
// Note: Expat has no API to determine whether a parser is a root parser,
// and since the Expat functions for defining the various maximum allowed
// amplifcation factors fail when a bad parser or a out-of-range factor
// is given without specifying which check failed, we check whether the
// factor is out-of-range to improve the error message. See also gh-90949.
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);
}
#endif

#if XML_COMBINED_VERSION >= 20702
/*[clinic input]
@permit_long_summary
Expand All @@ -1205,15 +1239,10 @@ pyexpat_xmlparser_SetAllocTrackerActivationThreshold_impl(xmlparseobject *self,
unsigned long long threshold)
/*[clinic end generated code: output=bed7e93207ba08c5 input=54182cd71ad69978]*/
{
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");
return set_activation_threshold(
self, cls, threshold,
XML_SetAllocTrackerActivationThreshold
);
}
#endif

Expand Down Expand Up @@ -1248,24 +1277,10 @@ pyexpat_xmlparser_SetAllocTrackerMaximumAmplification_impl(xmlparseobject *self,
float max_factor)
/*[clinic end generated code: output=6e44bd48c9b112a0 input=3544abf9dd7ae055]*/
{
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);
return set_maximum_amplification(
self, cls, max_factor,
XML_SetAllocTrackerMaximumAmplification
);
}
#endif

Expand Down
Loading