fix: add bounds-check guards for file reading functions#293
Merged
spe-ciellt merged 1 commit intogerbv:developfrom Feb 28, 2026
Merged
fix: add bounds-check guards for file reading functions#293spe-ciellt merged 1 commit intogerbv:developfrom
spe-ciellt merged 1 commit intogerbv:developfrom
Conversation
The mmap path in gerb_fopen() does not null-terminate the buffer, so strtol/strtod in gerb_fgetint() and gerb_fgetdouble() could read past the mapped region if a file ends with digits and no trailing newline. Copy mmap'd data into a heap buffer with a null terminator, matching the existing calloc path behavior. This also simplifies gerb_fclose() since fd->data is now always heap-allocated. Add early-return guards to gerb_fgetint() and gerb_fgetdouble() when the read pointer is at or past end-of-data. Guard the hex-float check in gerb_fgetdouble() against single-byte remaining buffers.
Contributor
|
I merge this patch for now, looks simple and on point. This patch makes the Linux file operations and Windows file operation more similar, so they could probably be more "the same". The reason for selecting the mmap()/unmap() in the beginning was that it made parsing faster. So for Unix like system I would still prefer the mmap()/unmap(). |
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.
Summary
gerb_file_thas two allocation paths ingerb_fopen():st_sizebytes with no null terminatordata[datalen]strtol()andstrtod()scan forward until they hit a non-matching character or'\0'. On the mmap path, if a file ends with digits and no trailing newline/command, these functions can read past the mapped region.Additionally,
gerb_fgetdouble()accessesstart[0]/start[1]for the hex-float guard (added in #285) without checking that at least 2 bytes remain.This PR:
'\0'terminator beforemunmap, making both code paths produce identical null-terminated buffersgerb_fclose()—fd->datais now always heap-allocated, so the#ifdef/munmapbranch is removedgerb_fgetint()andgerb_fgetdouble()when the read pointer is at or past end-of-datagerb_fgetdouble()withremaining >= 2before accessingstart[1]Affected functions
gerb_fopen()gerb_fclose()munmapbranch, useg_free()unconditionallygerb_fgetint()gerb_fgetdouble()Already safe (unchanged):
gerb_fgetc()(checksptr >= datalen),gerb_fgetstring()(usesiendbound).Testing
test-amacro-hex-expression.gbx) renders correctlytest-gerber-x2-attributes.gbx) renders correctly