Don't pre-allocate the Type1 /Subrs array from the declared count - #32062
Conversation
_parse_subrs allocated [None] * count from the count declared in the font before reading the body, so a malformed font declaring a huge count could force a large allocation before being rejected. Accumulate entries into a dict and build the list after the body parses, matching _parse_charstrings.
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process. You can also join us on discourse chat for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
| "Malformed Type1 font file: Incomplete /Subrs" | ||
| ) from None | ||
|
|
||
| array = [None] * count |
There was a problem hiding this comment.
If we are being more careful, we should also check that the length of entries == count and the maximum key is count -1.
|
Thank you @vsaraikin and congratulations on your first merged Matplotlib PR 🎉 I hope we hear from you agian. |
|
@meeseeksdev backport to v3.11.x |
…062-on-v3.11.x Backport PR #32062 on branch v3.11.x (Don't pre-allocate the Type1 /Subrs array from the declared count)
5 participants
Fixes #31962.
When parsing a Type1 font,
_parse_subrsread the declared/Subrscount and immediately didarray = [None] * count, before reading the body. The count comes straight from the font file, so a malformed font can declare a huge count in a handful of bytes and force a large allocation before anything is validated._parse_charstringsalready avoids this by accumulating into a dict, so this just brings_parse_subrsin line.The change accumulates the parsed entries into a dict and only allocates the
count-sized result list once the body has actually been read.The regression test builds a tiny font that declares
/Subrs 5000000 arraywith an empty body. On master that allocates ~40 MB ([None] * 5000000) before the parse fails; with the change the parse still fails the same way but peak allocation stays small. The test uses tracemalloc and asserts the peak stays under 5 MB.