From 7b42f540b40706647676c76734277499cbacd79d Mon Sep 17 00:00:00 2001 From: Joe Bryan Date: Fri, 28 Aug 2020 14:45:51 -0700 Subject: [PATCH] ur: fixes buffer over-read bugs in bitstream in tests --- pkg/urbit/tests/ur_tests.c | 6 +- pkg/urbit/ur/bitstream.c | 200 ++++++++++++++++++++++--------------- 2 files changed, 121 insertions(+), 85 deletions(-) diff --git a/pkg/urbit/tests/ur_tests.c b/pkg/urbit/tests/ur_tests.c index 95e84e89f..9ec4fecb5 100644 --- a/pkg/urbit/tests/ur_tests.c +++ b/pkg/urbit/tests/ur_tests.c @@ -1126,13 +1126,15 @@ _test_bsr64(void) static void _bsr_bytes_any_slow(ur_bsr_t *bsr, uint64_t len, uint8_t *out) { - uint64_t i, len_byt = len >> 3; + uint64_t i, len_byt = len >> 3, len_bit = ur_mask_3(len); for ( i = 0; i < len_byt; i++ ) { out[i] = _bsr8_any_slow(bsr, 8); } - out[len_byt] = _bsr8_any_slow(bsr, ur_mask_3(len)); + if ( len_bit ) { + out[len_byt] = _bsr8_any_slow(bsr, len_bit); + } } static int diff --git a/pkg/urbit/ur/bitstream.c b/pkg/urbit/ur/bitstream.c index fabc15adb..d5ff9642d 100644 --- a/pkg/urbit/ur/bitstream.c +++ b/pkg/urbit/ur/bitstream.c @@ -130,22 +130,22 @@ ur_bsr32_any(ur_bsr_t *bsr, uint8_t len) return 0; } else { - uint8_t off = bsr->off; - uint8_t rest = 8 - off; - const uint8_t *b = bsr->bytes; - uint32_t m = b[0] >> off; + uint8_t off = bsr->off; + uint8_t rest = 8 - off; + uint32_t m = bsr->bytes[0] >> off; if ( len < rest ) { bsr->off = off + len; return m & ((1 << len) - 1); } else { - uint8_t mask, len_byt; - uint32_t l; + const uint8_t *b; + uint8_t mask, len_byt; + uint32_t l; len -= rest; left--; - bsr->bytes++; + b = ++bsr->bytes; len_byt = len >> 3; @@ -164,33 +164,44 @@ ur_bsr32_any(ur_bsr_t *bsr, uint8_t len) mask = (1 << off) - 1; switch ( len_byt ) { + default: assert(0); + case 4: { - l = (uint32_t)b[1] - ^ (uint32_t)b[2] << 8 - ^ (uint32_t)b[3] << 16 - ^ (uint32_t)b[4] << 24; + l = (uint32_t)b[0] + ^ (uint32_t)b[1] << 8 + ^ (uint32_t)b[2] << 16 + ^ (uint32_t)b[3] << 24; } break; case 3: { - l = (uint32_t)b[1] - ^ (uint32_t)b[2] << 8 - ^ (uint32_t)b[3] << 16 - ^ (uint32_t)(b[4] & mask) << 24; + l = (uint32_t)b[0] + ^ (uint32_t)b[1] << 8 + ^ (uint32_t)b[2] << 16; + + if ( mask ) { + l ^= (uint32_t)(b[3] & mask) << 24; + } } break; case 2: { - l = (uint32_t)b[1] - ^ (uint32_t)b[2] << 8 - ^ (uint32_t)(b[3] & mask) << 16; + l = (uint32_t)b[0] + ^ (uint32_t)b[1] << 8; + + if ( mask ) { + l ^= (uint32_t)(b[2] & mask) << 16; + } } break; case 1: { - l = (uint32_t)b[1] - ^ (uint32_t)(b[2] & mask) << 8; + l = (uint32_t)b[0]; + + if ( mask ) { + l ^= (uint32_t)(b[1] & mask) << 8; + } } break; case 0: { - l = (uint32_t)(b[1] & mask); + l = ( mask ) ? (uint32_t)(b[0] & mask) : 0; } break; } @@ -212,22 +223,22 @@ ur_bsr64_any(ur_bsr_t *bsr, uint8_t len) return 0; } else { - uint8_t off = bsr->off; - uint8_t rest = 8 - off; - const uint8_t *b = bsr->bytes; - uint64_t m = b[0] >> off; + uint8_t off = bsr->off; + uint8_t rest = 8 - off; + uint64_t m = bsr->bytes[0] >> off; if ( len < rest ) { bsr->off = off + len; return m & ((1 << len) - 1); } else { - uint8_t mask, len_byt; - uint64_t l; + const uint8_t *b; + uint8_t mask, len_byt; + uint64_t l; len -= rest; left--; - bsr->bytes++; + b = ++bsr->bytes; len_byt = len >> 3; @@ -247,74 +258,95 @@ ur_bsr64_any(ur_bsr_t *bsr, uint8_t len) switch ( len_byt ) { case 8: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)b[3] << 16 - ^ (uint64_t)b[4] << 24 - ^ (uint64_t)b[5] << 32 - ^ (uint64_t)b[6] << 40 - ^ (uint64_t)b[7] << 48 - ^ (uint64_t)b[8] << 56; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8 + ^ (uint64_t)b[2] << 16 + ^ (uint64_t)b[3] << 24 + ^ (uint64_t)b[4] << 32 + ^ (uint64_t)b[5] << 40 + ^ (uint64_t)b[6] << 48 + ^ (uint64_t)b[7] << 56; } break; case 7: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)b[3] << 16 - ^ (uint64_t)b[4] << 24 - ^ (uint64_t)b[5] << 32 - ^ (uint64_t)b[6] << 40 - ^ (uint64_t)b[7] << 48 - ^ (uint64_t)(b[8] & mask) << 56; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8 + ^ (uint64_t)b[2] << 16 + ^ (uint64_t)b[3] << 24 + ^ (uint64_t)b[4] << 32 + ^ (uint64_t)b[5] << 40 + ^ (uint64_t)b[6] << 48; + + if ( mask ) { + l ^= (uint64_t)(b[7] & mask) << 56; + } } break; case 6: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)b[3] << 16 - ^ (uint64_t)b[4] << 24 - ^ (uint64_t)b[5] << 32 - ^ (uint64_t)b[6] << 40 - ^ (uint64_t)(b[7] & mask) << 48; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8 + ^ (uint64_t)b[2] << 16 + ^ (uint64_t)b[3] << 24 + ^ (uint64_t)b[4] << 32 + ^ (uint64_t)b[5] << 40; + + if ( mask ) { + l ^= (uint64_t)(b[6] & mask) << 48; + } } break; case 5: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)b[3] << 16 - ^ (uint64_t)b[4] << 24 - ^ (uint64_t)b[5] << 32 - ^ (uint64_t)(b[6] & mask) << 40; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8 + ^ (uint64_t)b[2] << 16 + ^ (uint64_t)b[3] << 24 + ^ (uint64_t)b[4] << 32; + + if ( mask ) { + l ^= (uint64_t)(b[5] & mask) << 40; + } } break; case 4: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)b[3] << 16 - ^ (uint64_t)b[4] << 24 - ^ (uint64_t)(b[5] & mask) << 32; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8 + ^ (uint64_t)b[2] << 16 + ^ (uint64_t)b[3] << 24; + + if ( mask ) { + l ^= (uint64_t)(b[4] & mask) << 32; + } } break; case 3: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)b[3] << 16 - ^ (uint64_t)(b[4] & mask) << 24; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8 + ^ (uint64_t)b[2] << 16; + + if ( mask ) { + l ^= (uint64_t)(b[3] & mask) << 24; + } } break; case 2: { - l = (uint64_t)b[1] - ^ (uint64_t)b[2] << 8 - ^ (uint64_t)(b[3] & mask) << 16; + l = (uint64_t)b[0] + ^ (uint64_t)b[1] << 8; + + if ( mask ) { + l ^= (uint64_t)(b[2] & mask) << 16; + } } break; case 1: { - l = (uint64_t)b[1] - ^ (uint64_t)(b[2] & mask) << 8; + l = (uint64_t)b[0]; + + if ( mask ) { + l ^= (uint64_t)(b[1] & mask) << 8; + } } break; case 0: { - l = (uint64_t)(b[1] & mask); + l = ( mask ) ? (uint64_t)(b[0] & mask) : 0; } break; } @@ -925,19 +957,21 @@ _bsw_bytes_unsafe(ur_bsw_t *bsw, uint64_t len, uint8_t *byt) m = byt[i] >> rest; } - if ( len_bit < rest ) { - l = byt[len_byt] & ((1 << len_bit) - 1); - bsw->bytes[fill] = m ^ (l << off); - off += len_bit; - } - else { - l = byt[len_byt] & mask; - bsw->bytes[fill++] = m ^ (l << off); + if ( len_bit ) { + if ( len_bit < rest ) { + l = byt[len_byt] & ((1 << len_bit) - 1); + bsw->bytes[fill] = m ^ (l << off); + off += len_bit; + } + else { + l = byt[len_byt] & mask; + bsw->bytes[fill++] = m ^ (l << off); - m = byt[len_byt] >> rest; + m = byt[len_byt] >> rest; - off = len_bit - rest; - bsw->bytes[fill] = m & ((1 << off) - 1); + off = len_bit - rest; + bsw->bytes[fill] = m & ((1 << off) - 1); + } } }