Repository navigation
Fix UTF-8 buffer overflows from code-point string lengths - #522
Open
JarrettSJohnson wants to merge 2 commits into
Open
JarrettSJohnson wants to merge 2 commits into
JarrettSJohnson wants to merge 2 commits into
Conversation
PyString_Size maps to PyUnicode_GetLength, which counts code points, but these buffers are filled with UTF-8 bytes. Non-ASCII wizard prompts and popup menu labels overflowed the heap. Fixes #517 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The message wizard prints its text, which raises UnicodeEncodeError on the cp1252 console used by Windows CI before the prompt conversion is reached. Set the prompt on a plain Wizard instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
speleo3
reviewed
Oct 8, 2026
| str = PyString_AsString(elem); | ||
| cl = l; | ||
| l = strlen(str); /* buffer size in UTF-8 bytes */ | ||
| cl = PyString_Size(elem); /* display width in characters */ |
Contributor
There was a problem hiding this comment.
Maybe better to remove the legacy macros and to use PyUnicode_GetLength directly?
pymol-open-source/layer0/os_python.h
Lines 47 to 48 in edcda80
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #517
Problem
On Python 3,
PyString_Sizemaps toPyUnicode_GetLength, which counts code points. Three places used that count to size acharbuffer and then copied the UTF-8 bytes into it. A CJK character is one code point but three bytes, so non-ASCII text overran the buffer:PConvPyListToStringVLA(layer1/PConv.cpp): heap overflow.WizardRefreshuses it for the wizard prompt, which is the crash in the issue.PopUpNew(layer4/PopUp.cpp): heap overflow. Menu label and command rows were sized by code points and filled withstrcpy.PConvPyListToStrVLAList(layer1/PConv.cpp): same sizing, butUtilNCopytruncates, so non-ASCII strings were cut short, possibly mid-character. Nothing in the tree calls it.Fix
Size the buffers by UTF-8 byte length (
SomeString::length()/strlen). The popup width still uses the code-point count, because the GLUT font draws one glyph per code point (FontGLUT.cpp). Non-ASCII labels don't make the popup wider.Not covered: a
strthat can't be encoded as UTF-8 (e.g. a lone surrogate"\ud800") makesPyUnicode_AsUTF8return null, andPConvPyListToStringVLAwould dereference it. That bug already existed and is separate.Testing
testing/tests/wizard/prompt.py: sets a long CJK wizard prompt and refreshes it. Without the fix it crashes (heap corruption abort); with the fix it passes. It uses a plainWizardbecause the message wizard prints its text, which fails on the cp1252 console used by Windows CI.tests/wizard/andtests/api/viewing.pyviatesting.py: 52 tests, no failures (6 skipped: no GUI / TODO).🤖 Generated with Claude Code