xls: fix panic parsing formulas in BIFF5 workbooks#643
Open
mogery wants to merge 3 commits intotafia:masterfrom
Open
xls: fix panic parsing formulas in BIFF5 workbooks#643mogery wants to merge 3 commits intotafia:masterfrom
mogery wants to merge 3 commits intotafia:masterfrom
Conversation
parse_formula decoded the non-3D PtgRef (0x24/0x44/0x64), PtgArea (0x25/0x45/0x65), PtgRefErr (0x2A/0x4A/0x6A), and PtgAreaErr (0x2B/0x4B/0x6B) tokens assuming BIFF8 sizes (4 and 8 data bytes respectively). In BIFF2-5 those tokens use 1-byte columns, so their data blocks are 3 and 6 bytes. When a BIFF5 workbook hit one of these tokens, the parser advanced rgce by too many bytes, mis-identified following bytes as a new Ptg, and eventually panicked with "range end index 2 out of range for slice of length 0" inside utils::read_u16 - taking down any caller that opened the file. Thread biff through parse_formula and branch the four handlers on the workbook's BIFF version, mirroring how parse_defined_names and the 3D Ptg handlers already do it. Made-with: Cursor
Made-with: Cursor
Author
|
Apologies, fixed the formatting. |
Collaborator
|
Thanks. Could you also add a test case to the |
Author
|
Added a test with a minimal fixture file. Panics on master, works on this branch, opens in Excel cleanly. |
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
2 participants
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.
parse_formula decoded the non-3D PtgRef (0x24/0x44/0x64), PtgArea (0x25/0x45/0x65), PtgRefErr (0x2A/0x4A/0x6A), and PtgAreaErr (0x2B/0x4B/0x6B) tokens assuming BIFF8 sizes (4 and 8 data bytes respectively). In BIFF2-5 those tokens use 1-byte columns, so their data blocks are 3 and 6 bytes. When a BIFF5 workbook hit one of these tokens, the parser advanced rgce by too many bytes, mis-identified following bytes as a new Ptg, and eventually panicked with
"range end index 2 out of range for slice of length 0" inside utils::read_u16 - taking down any caller that opened the file.
Thread biff through parse_formula and branch the four handlers on the workbook's BIFF version, mirroring how parse_defined_names and the 3D Ptg handlers already do it.
Example file that caused a panic without this patch: https://www.asx.com.au/content/dam/asx/participants/derivatives-market/equity-derivatives/equity-derivatives-statistics/2016/annual-market-summary-2016.xls
PR authored with Claude Opus 4.7