Skip to content

Fix BASE table parsing infinite loop memory crash#1675

Merged
cmyr merged 3 commits into
googlefonts:mainfrom
daltonmaag:base-blackhole
Oct 3, 2025
Merged

Fix BASE table parsing infinite loop memory crash#1675
cmyr merged 3 commits into
googlefonts:mainfrom
daltonmaag:base-blackhole

Conversation

@RickyDaMa

Copy link
Copy Markdown
Contributor

When parsing MinMax records, the token declaring the record isn't consumed, and so the parser immediately starts creating another MinMax record, but the token declaring the record isn't consumed, and so the parser immediately starts creating another MinMax record, but the token declaring the record isn't consumed, and so the parser immediately starts creating another MinMax record, but the token declaring the record isn't consumed, and so the parser immediately starts creating another MinMax record, but the token declaring the record isn't consumed, and so the parser immediately starts creating another MinMax record, but the token declaring the record isn't consumed, and so the parser immediately starts creating another MinMax record—

Killed out of memory.

Instead, we now consume the MinMax token so the parser actually progresses. Also speculatively fixes this same pattern in some GDEF table parsing code

RickyDaMa and others added 2 commits October 2, 2025 17:27
Co-authored-by: Harry Dalton <harry.dalton@daltonmaag.com>
Co-authored-by: Harry Dalton <harry.dalton@daltonmaag.com>
@Hoolean

Hoolean commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

(We've made an issue for full compilation in the future, too: #1676 😃)

@cmyr

cmyr commented Oct 2, 2025

Copy link
Copy Markdown
Member

Can we add a test case in fea-rs/test-data/parse-tests/good? See this README for a quick explanation of how these tests work.

@RickyDaMa

Copy link
Copy Markdown
Contributor Author

Test added ✅

@cmyr cmyr left a comment

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.

Thanks!

@cmyr cmyr added this pull request to the merge queue Oct 3, 2025
Merged via the queue into googlefonts:main with commit d8ddf70 Oct 3, 2025
12 checks passed
@RickyDaMa RickyDaMa deleted the base-blackhole branch October 6, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants