bitvec_read_field(): fix incorrect bit-shift issue found by UBSan
While running a sanitized version of the bitvec_test I get:
bitvec.c:492:24: runtime error: shift exponent 64 is too large
for 64-bit type 'long unsigned int'
This error is triggered by the following line in the bitvec_test:
_bitvec_read_field(0, 8 * 8 + 1); /* too many bits */
which basically tries to parse more bits (65) than the test vector
actually has (64). The problem is that we don't check if the
given vector has enough data *before* entering the parsing loop,
so we end up doing weird bit-shifts and getting weird values:
bitvec_read_field(idx=0, len=65) => bd5b7ddffdd7b5db (error)
Unfortunately, this problem remained unnoticed so far because in
'tests/testsuite.at' we don't check if stderr is empty. This is
fixed in a follow up change [1].
Rather than checking for errors in every loop iteration, do this
once and return early if the overrun is possible with the given
offset and length arguments.
Change-Id: I4deeabba7ebb720cdbe7c85b37bc011d05bdfa65
Related: [1] Ia82b92eddb18dc596881abcef2f098dc7385538b
diff --git a/src/bitvec.c b/src/bitvec.c
index b411a72..2303a0d 100644
--- a/src/bitvec.c
+++ b/src/bitvec.c
@@ -480,15 +480,18 @@
{
unsigned int i;
uint64_t ui = 0;
+
+ /* Prevent bitvec overrun due to incorrect index and/or length */
+ if (len && bytenum_from_bitnum(*read_index + len - 1) >= bv->data_len) {
+ errno = EOVERFLOW;
+ return 0;
+ }
+
bv->cur_bit = *read_index;
errno = 0;
for (i = 0; i < len; i++) {
int bit = bitvec_get_bit_pos((const struct bitvec *)bv, bv->cur_bit);
- if (bit < 0) {
- errno = -bit;
- break;
- }
if (bit)
ui |= ((uint64_t)1 << (len - i - 1));
bv->cur_bit++;