Skip to content

Fix potential buffer overflo when using XADRegex#177

Merged
PaulTaykalo merged 6 commits intomasterfrom
fix/null-potential-buffer-overflow-in-regex
Apr 16, 2026
Merged

Fix potential buffer overflo when using XADRegex#177
PaulTaykalo merged 6 commits intomasterfrom
fix/null-potential-buffer-overflow-in-regex

Conversation

@PaulTaykalo
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a potential out-of-bounds read / buffer overflow risk when using XADRegex to match against NSData buffers by ensuring the regex engine always receives a NUL-terminated byte buffer and by sanitizing invalid input ranges.

Changes:

  • Add nullterminateddata storage and pass it to regexec to guarantee NUL-terminated input.
  • Clamp NSRange inputs to valid bounds and handle nil NSData inputs safely.
  • Add XCTest coverage for invalid/overflowing ranges and mutable-data snapshot behavior.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
XADRegex.m Creates and uses a NUL-terminated buffer for regexec, clamps ranges, and manages added buffer lifetime.
XADRegex.h Adds an ivar to retain the NUL-terminated buffer alongside the original data snapshot.
XADMasterTests/XADRegexTests.m Adds unit tests covering invalid ranges and ensuring stable matching when input data is mutated after matching begins.
XADMaster.xcodeproj/project.pbxproj Registers the new regex test file in the test target build phases/groups.
.gitignore Ignores a local DerivedData build output path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread XADRegex.m Outdated
Comment on lines +159 to +164
currdata=[data copy];

[nullterminateddata release];
NSMutableData *terminated=[NSMutableData dataWithData:currdata];
[terminated appendBytes:"\0" length:1];
nullterminateddata=[terminated retain];
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

beginMatchingData:range: currently copies the entire input twice for mutable inputs (first via [data copy] into currdata, then again via [NSMutableData dataWithData:currdata] to create the null-terminated buffer). For large payloads this can significantly increase peak memory usage and may lead to avoidable memory pressure/OOM. Consider allocating a single buffer of datalength + 1 bytes, copying the source bytes once, and then (a) using that same backing store for currdata (length datalength, no free-on-done) and (b) keeping the owning NSMutableData as nullterminateddata for regexec.

Suggested change
currdata=[data copy];
[nullterminateddata release];
NSMutableData *terminated=[NSMutableData dataWithData:currdata];
[terminated appendBytes:"\0" length:1];
nullterminateddata=[terminated retain];
[nullterminateddata release];
NSMutableData *terminated=[[NSMutableData alloc] initWithLength:datalength+1];
void *terminatedbytes=[terminated mutableBytes];
if(datalength>0) memcpy(terminatedbytes,[data bytes],datalength);
((char *)terminatedbytes)[datalength]='\0';
currdata=[[NSData alloc] initWithBytesNoCopy:terminatedbytes length:datalength freeWhenDone:NO];
nullterminateddata=terminated;

Copilot uses AI. Check for mistakes.
@PaulTaykalo PaulTaykalo merged commit 1fb2c05 into master Apr 16, 2026
3 checks passed
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.

2 participants