Skip to content

Conversation

hartwork
Copy link
Contributor

@hartwork hartwork commented Oct 3, 2025

Expose the XML Expat 2.7.2 mitigation APIs to disallow use of disproportional amounts of dynamic memory from within an Expat parser (see CVE-2025-59375 for instance).

The exposed APIs are available on Expat parsers, that is, parsers created by xml.parsers.expat.ParserCreate(), as:

  • parser.SetAllocTrackerActivationThreshold(threshold), and
  • parser.SetAllocTrackerMaximumAmplification(max_factor).

(cherry picked from commits f04bea4 and 68a1778)

CC @picnixz

picnixz and others added 4 commits October 3, 2025 01:42
…2025-59375) (python#139234)

Expose the XML Expat 2.7.2 mitigation APIs to disallow use of
disproportional amounts of dynamic memory from within an Expat
parser (see CVE-2025-59375 for instance).

The exposed APIs are available on Expat parsers, that is,
parsers created by `xml.parsers.expat.ParserCreate()`, as:

- `parser.SetAllocTrackerActivationThreshold(threshold)`, and
- `parser.SetAllocTrackerMaximumAmplification(max_factor)`.

(cherry picked from commit f04bea4)
…on API (python#139366)

Fix some typos left in f04bea4,
and simplify some internal functions to ease maintenance of future
mitigation APIs.

(cherry picked from commit 68a1778)
@hartwork hartwork requested a review from picnixz October 3, 2025 15:29
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Thanks for the backport!

#include <stdbool.h>
#include "structmember.h" // PyMemberDef
#include "frameobject.h"
#include <stddef.h> // offsetof()
Copy link
Member

Choose a reason for hiding this comment

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

Do we need offsetof?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@picnixz we do!

For one, because it is used in:

static PyMemberDef xmlparse_members[] = {
    {"intern", T_OBJECT, offsetof(xmlparseobject, intern), READONLY, NULL},
    {NULL}
};

For two, because it is added in de690f1, the source of the backport.

Copy link
Member

Choose a reason for hiding this comment

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

Hum. Why did add it? I think it was because my IDE complained at some point. Ok then.

Copy link
Contributor Author

@hartwork hartwork Oct 3, 2025

Choose a reason for hiding this comment

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

@picnixz I need to correct me earlier statement: I got the commit wrong, it's actually f04bea4 and you did not add that import there, it was already present in main and 3.14 and 3.13. So the question now seems to be whether we want to add or not add that import in the backports targetting 3.12, 3.11 and 3.10. It's the correct header but then some other header must be already pulling it in or the code would not compile already prior to these backports. I'm good with dropping or keeping that new include — what would you prefer?

Copy link
Member

@picnixz picnixz Oct 3, 2025

Choose a reason for hiding this comment

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

Try to reduce the diff as much as possible. If it works without the include then let's not add it. I think it would make better sense to remove it from 3.13 and 3.14 actually but I don't know if other files actually have an explicit or an implicit include policy...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try to reduce the diff as much as possible. If it works without the include then let's not add it.

@picnixz okay, let me try…

I think it would make better sense to remove it from 3.13 and 3.14 actually but I don't know if other files actually have an explicit or an implicit include policy...

That I would have trouble supporting. Please don't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@picnixz PS: I found…

Doc/c-api/structures.rst:   (You may need to ``#include <stddef.h>`` for :c:func:`!offsetof`.)

…and also…

Include/structmember.h:#include <stddef.h> /* For offsetof (not always provided by Python.h) */

in 3.12 code and that all of 3.10, 3.11, 3.12 have pyexpat.c include structmember.h but 3.13, 3.14, main do not and so these seem to rightfully contain a dedicated #include <stddef.h> for offsetof.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@picnixz okay, let me try…

Update: dropped from all of 3.10, 3.11, 3.12 backports by now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants