Skip to content

gh-156939: Detect buffer overflow in PyBytesWriter in debug mode - #156943

Open
vstinner wants to merge 4 commits into
python:mainfrom
vstinner:writer_canary
Open

gh-156939: Detect buffer overflow in PyBytesWriter in debug mode#156943
vstinner wants to merge 4 commits into
python:mainfrom
vstinner:writer_canary

Conversation

@vstinner

@vstinner vstinner commented Sep 4, 2026

Copy link
Copy Markdown
Member

Reserve one byte in PyBytesWriter used as a canary byte: set it to a special value. PyBytesWriter_Finish() checks if the canary byte has been overriden. Add a test on the feature.

Update buffer overflow check in fcntl: allocate extra guard bytes in the writer and then truncate these bytes.

@vstinner

vstinner commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Oh, the test_xml_etree_c failure is a legit failure: unicode_encode_ucs1() calls with errors="xmlcharrefreplace" writes a NUL byte after the writer buffer. xmlcharrefreplace() calls sprintf(str, "&#%d;", PyUnicode_READ(kind, data, i)) which writes a final NUL byte.

@vstinner

vstinner commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

The test_struct error is also a legit bug: a Pascal string of 0 characters writes a NUL byte into an empty buffer, and so triggers a buffer overflow.

@vstinner

vstinner commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

On Ubuntu, 8 tests are failing:

8 tests failed:
    test_codeccallbacks test_fcntl test_minidom test_sax test_struct
    test_urllib test_xml_etree test_xml_etree_c

Reserve one byte in PyBytesWriter used as a canary byte: set it to a
special value. PyBytesWriter_Finish() checks if the canary byte has
been overriden. Add a test on the feature.

Update buffer overflow check in fcntl: allocate extra guard bytes in
the writer and then truncate these bytes.
@vstinner

vstinner commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

I fixed struct.pack() in PR gh-157071 and xmlcharrefreplace in PR gh-157109.

Avoid calling PyBytesWriter_Grow().
@vstinner
vstinner marked this pull request as ready for review September 9, 2026 20:42
@vstinner

vstinner commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

cc @serhiy-storchaka

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.

1 participant