Repository navigation
Segfault/UB from expat when re-entering the XML Parser #146169
Description
Crash report
What happened?
Semi-reliable(Interstingly, python in macos doesn't segfault for me, but docker/linux aarch64 does reliably) crash with this code:
from xml.parsers import expat
p = expat.ParserCreate(encoding="utf-16")
def start(name, attrs):
p.CharacterDataHandler = lambda data: p.Parse(data, 0)
p.StartElementHandler = start
data = b"\xff\xfe<\x00a\x00>\x00x\x00"
for i in range(len(data)):
try:
p.Parse(data[i:i+1], i == len(data) - 1)
except Exception:
passThis code /is/ doing some pretty naughty stuff, but the main problem seems to be that the handler is being set to re-enter the parser. The expat docs do say:
To state the obvious: the three parsing functions XML_Parse, XML_ParseBuffer and XML_GetBuffer must not be called from within a handler unless they operate on a separate parser instance, that is, one that did not call the handler. For example, it is OK to call the parsing functions from within an XML_ExternalEntityRefHandler, if they apply to the parser created by XML_ExternalEntityParserCreate.
and I see that the python expat parser code tracks in_callback:
Line 91 in 52c0186
| int in_callback; /* Is a callback active? */ |
So I wonder if we can avoid the segfault by preventing Parse calls when in_callback==true?
There's also a secondary issue in play here, that Parse() seems to call
XML_SetEncoding
Line 872 in 52c0186
| (void)XML_SetEncoding(self->itself, "utf-8"); |
Without the check outlined in the expat docs:
Set the encoding to be used by the parser. It is equivalent to passing a non-NULL encoding argument to the parser creation functions. It must not be called after XML_Parse or XML_ParseBuffer have been called on the given parser. Returns XML_STATUS_OK on success or XML_STATUS_ERROR on error.
This is almost definitely not going to cause issues unless the encoding is actually changing, (not that common) at which point the UB will rear its head as the internal state of the parser becomes inconsistent.
Dockerfile reproducer
ARG REPO=https://github.com/python/cpython.git
ARG BRANCH=main
FROM ubuntu:24.04
ARG REPO
ARG BRANCH
ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y \
build-essential git pkg-config \
libssl-dev libbz2-dev libreadline-dev libsqlite3-dev \
liblzma-dev libffi-dev zlib1g-dev uuid-dev \
&& rm -rf /var/lib/apt/lists/*
RUN git clone --branch ${BRANCH} --depth 1 \
${REPO} /cpython
RUN cd /cpython && \
./configure --prefix=/python --without-ensurepip && \
make -j$(nproc) && \
make install
# ── TEST SCRIPT ──────────────────────────────────────────────────
RUN cat > /test.py << 'EOF'
from xml.parsers import expat
p = expat.ParserCreate(encoding="utf-16")
def start(name, attrs):
p.CharacterDataHandler = lambda data: p.Parse(data, 0)
p.StartElementHandler = start
data = b"\xff\xfe<\x00a\x00>\x00x\x00"
for i in range(len(data)):
try:
p.Parse(data[i:i+1], i == len(data) - 1)
except Exception:
pass
EOF
# ──────────────────────────────────────────────────────────────────────────────
CMD ["/bin/sh", "-c", "uname -m && /python/bin/python3 -VV && /python/bin/python3 /test.py"]
Gives on my pc:
docker run --rm -it expattest
aarch64
Python 3.15.0a7+ (heads/main:52c0186, Mar 19 2026, 13:06:19) [GCC 13.3.0]
52c01864c4778a351e5aa3584e86ed6fd212a5a4
Segmentation fault (core dumped)
CPython versions tested on:
CPython main branch
Operating systems tested on:
macOS
Output from running 'python -VV' on the command line:
Python 3.15.0a7+ (heads/main:52c0186, Mar 19 2026, 13:06:19) [GCC 13.3.0]
Activity
stestagg commentedon Mar 19, 2026
Yeah, that sounds sensible, unfortunately I don't know enough about expat, or how python uses it, to know the full ramifications of that change
[-]Segfault/UB from expat when re-entering the Parser[/-][+]Segfault/UB from expat when re-entering the XML Parser[/+]picnixz commentedon Mar 20, 2026
Considering I do not want PRs generated by AI and then reviews where I talk to an AI, I will fix those issues when I come back from my travels, namely next week.
Thanks for the report and reproducers though!
picnixz commentedon Mar 28, 2026
So as you said, there are two issues:
I will make two different PRs. Note that for Parse(), we do have a weird way to handle that for the CharacterDataHandler which I don't remember why we added this:
in_callbackis only ever used here and inpyexpat_xmlparser_GetInputContext_implwhich only makes sense when we're in a callback. All other accesses toin_callbackare write accesses.From a FT-POV as well, I think we likely have concurrency issues despite the use of
{Py_mod_gil, Py_MOD_GIL_NOT_USED}sinceself->in_callbackcould be called set concurrently and modified concurrently as well but I'm not entirely sure on that point.@ZeroIntensity What's your insight on FT-concerns for PyExpat? should we just use the GIL in the meantime?
picnixz commentedon Mar 28, 2026
Ok, after fiddling a bit more with the reproducer:
pythongh-146169: correctly handle re-entrant parsing calls in Expat h…
hartwork commentedon Jun 25, 2026
@stestagg @picnixz FYI Expat >=2.8.2 now protects against this form of parser re-entry by itself. The related pull requests and CVEs are:
XML_ResumeParserfrom being called from a handler libexpat/libexpat#1267XML_TOK_DATA_CHARShandler calls indoCdataSectionlibexpat/libexpat#1278expat: update to 2.8.2.
ZeroIntensity commentedon Jun 26, 2026
Sorry, missed this. We really don't want to re-enable the GIL for any module. Are the Python objects thread-unsafe, or the Expat library as a whole?
Pullup ticket #7146 - requested by taca
Pullup ticket #7146 - requested by taca