Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit badb3ce

Browse files
JohanEngelendlang-bot
authored andcommittedMar 4, 2022
Prevent buffer overread in core.internal.gc.bits
Bug found with AddressSanitizer, for the unittest with this code: ``` GCBits bits; bits.alloc(10000); auto data = bits.data; GCBits src; src.alloc(67); src.data[0] = 0x4; bits.copyRangeRepeating(2, 10000, src.data, 67); <---- triggers the bug ``` The bug happens for the loop iteration in copyRangeRepeating that calls copyRange where these are the values of variables in copyRange: cntWords = 2 target = 1342 last = 1408 lastWord = 22 lastOff = 0 firstOff = 62 len = 67 Overread happens on `source[cntWords]`. Proof of correctness of this fix: Let's assume `firstOff == 3`, then `source[cntWords] << firstOff` has value `0bxx...xxxxx000` in bits. For `lastOff == 2`, then `mask = (BITS_2 << lastOff) - 1 == (2 << lastOff) - 1 == 0b1000 - 1 == 0b111`. `src&mask` would always be 0. For `lastOff == 3`, then `mask = 0b1111`. `src&mask` would be 0bx000, i.e. actual bits of the value read would be used.
1 parent 402d1a3 commit badb3ce

File tree

1 file changed

+3
-1
lines changed

1 file changed

+3
-1
lines changed
 

‎src/core/internal/gc/bits.d

+3-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,9 @@ struct GCBits
239239
size_t cntWords = lastWord - firstWord;
240240
copyWordsShifted(firstWord, cntWords, firstOff, source);
241241

242-
wordtype src = (source[cntWords - 1] >> (BITS_PER_WORD - firstOff)) | (source[cntWords] << firstOff);
242+
wordtype src = (source[cntWords - 1] >> (BITS_PER_WORD - firstOff));
243+
if (lastOff >= firstOff) // prevent buffer overread
244+
src |= (source[cntWords] << firstOff);
243245
wordtype mask = (BITS_2 << lastOff) - 1;
244246
data[lastWord] = (data[lastWord] & ~mask) | (src & mask);
245247
}

0 commit comments

Comments
 (0)
Please sign in to comment.