Skip to content

CFData: fix CFDataReplaceBytes length miscalculation (heap overflow)#54

Open
DTW-Thalion wants to merge 1 commit into
gnustep:masterfrom
DTW-Thalion:fix/cfdata-replacebytes-overflow
Open

CFData: fix CFDataReplaceBytes length miscalculation (heap overflow)#54
DTW-Thalion wants to merge 1 commit into
gnustep:masterfrom
DTW-Thalion:fix/cfdata-replacebytes-overflow

Conversation

@DTW-Thalion

@DTW-Thalion DTW-Thalion commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

CFDataReplaceBytes computes the new buffer length as
range.location + newLength, which ignores the bytes after the replaced
range. When a replace or delete leaves a non-empty tail, the buffer is grown
too small and the subsequent tail memmove writes past the allocation
(heap buffer overflow), and the stored length is wrong (the tail is dropped).

This affects CFDataReplaceBytes, CFDataDeleteBytes, and any replace that
leaves trailing bytes — ordinary CFMutableData usage.

Reproduction (AddressSanitizer)

CFMutableDataRef d = CFDataCreateMutable(NULL, 0);
UInt8 a[20]; CFDataAppendBytes(d, a, 20);            /* length 20 */
UInt8 b[30]; CFDataReplaceBytes(d, CFRangeMake(2,3), b, 30);
/* correct new length = 20 - 3 + 30 = 47 */
  • Before: ASan reports heap-buffer-overflow WRITE of size 15 in memmove
    via CFDataReplaceBytes (the buffer is grown to only 32 bytes in
    CFDataCheckCapacityAndGrow); CFDataGetLength returns 32 instead of 47.
  • After: length 47, no overflow.

Fix

Compute the new length as _length - range.length + newLength, and base the
range precondition (assert) and the tail-move guard on _length rather than
_capacity / the miscomputed length.

Test

Tests/CFData/replacebytes.m covers a middle grow and a middle delete,
asserting both the resulting length and that the prefix/tail are preserved.

@DTW-Thalion DTW-Thalion force-pushed the fix/cfdata-replacebytes-overflow branch from 7d04fd8 to cc65658 Compare June 28, 2026 18:39
CFDataReplaceBytes computed the new buffer length as
range.location + newLength, which ignores the bytes following the
replaced range.  When a replace or delete leaves a non-empty tail, the
buffer was grown too small and the tail memmove wrote past the
allocation (heap buffer overflow), and the stored length was wrong (the
tail was dropped).

Compute the length as _length - range.length + newLength, and base the
range precondition and the tail-move guard on _length rather than
_capacity / the miscomputed length.

Add Tests/CFData/replacebytes.m covering a middle grow and a middle
delete.
@DTW-Thalion DTW-Thalion force-pushed the fix/cfdata-replacebytes-overflow branch from cc65658 to 9ced23d Compare July 3, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant