Skip to content

Don't pre-allocate the Type1 /Subrs array from the declared count #32062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vsaraikin wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −2
Open

Don't pre-allocate the Type1 /Subrs array from the declared count #32062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions lib/matplotlib/_type1font.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,13 @@ def _parse_subrs(self, tokens, _data):
f"Token following /Subrs must be a number, was {count_token}"
)
count = count_token.value()
array = [None] * count
next(t for t in tokens if t.is_keyword('array'))
# Accumulate the parsed subrs into a dict and only allocate the result
# list once the body has been read. Allocating ``[None] * count`` up
# front lets a malformed font declare a huge count in a few bytes and
# force a large allocation before it is rejected; _parse_charstrings
# already avoids this by accumulating into a dict.
Comment on lines +619 to +620

@timhoffm timhoffm Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# force a large allocation before it is rejected; _parse_charstrings
# already avoids this by accumulating into a dict.
# force a large allocation before it is rejected

This is not relevant here. Also (without being expert in this part of the codebase), _parse_charstrings actually returns a dict, so accumulting in the dict is natural there. Whereas here it an explicit measure to prevent pre-allocation.

entries = {}
for _ in range(count):
next(t for t in tokens if t.is_keyword('dup'))
index_token = next(tokens)
Expand All @@ -635,7 +640,11 @@ def _parse_subrs(self, tokens, _data):
f"was {token}"
)
binary_token = tokens.send(1+nbytes_token.value())
array[index_token.value()] = binary_token.value()
entries[index_token.value()] = binary_token.value()

array = [None] * count
for index, value in entries.items():
array[index] = value

return array, next(tokens).endpos()

Expand Down
24 changes: 24 additions & 0 deletions lib/matplotlib/tests/test_type1font.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,27 @@ def test_encrypt_decrypt_roundtrip():
decrypted = t1f.Type1Font._decrypt(encrypted, 'eexec')
assert encrypted != decrypted
assert data == decrypted


def test_Subrs_no_preallocation(tmp_path):
# Regression test for #31962: a font declaring a huge /Subrs count must not
# cause a large allocation sized from that (untrusted) count before the
# body is parsed. Here the body is empty, so parsing must fail without
# first allocating a count-sized array.
import tracemalloc
count = 5_000_000
plaintext = (b'/FontName /X def\n/FontBBox [0 0 1 1] def\n'
+ f'/Subrs {count} array\n'.encode())
enc = t1f.Type1Font._encrypt(plaintext, 'eexec').hex().encode()
pfa = (b'%!PS-AdobeFont-1.0: X 001.000\neexec\n' + enc + b'\n'
+ b'0' * 512 + b'\ncleartomark\n')
path = tmp_path / 'x.pfa'
path.write_bytes(pfa)

tracemalloc.start()
with pytest.raises(StopIteration):

@timhoffm timhoffm Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Am I correct that this StopIteration is raised from one of the next() calls in the for-loop?

Should we catch that and raise a more appropriate error such as

try:
    for _ in range(count):
        ...
except StopIteration:
    raise RuntimeError("Malformed Type1 font file: Incomplete /Subrs")

t1f.Type1Font(str(path))
_, peak = tracemalloc.get_traced_memory()
tracemalloc.stop()
# The pre-allocation regressed to ~40 MB ([None] * count) before failing.
assert peak < 5 * 1024 * 1024
Loading