reftable/record: handle overflows when decoding varints
The logic to decode varints isn't able to detect integer overflows: as long as the buffer still has more data available, and as long as the current byte has its 0x80 bit set, we'll continue to add up these values to the result. This will eventually cause the `uint64_t` to overflow, at which point we'll return an invalid result. Refactor the function so that it is able to detect such overflows. The implementation is basically copied from Git's own `decode_varint()`, which already knows to handle overflows. The only adjustment is that we also take into account the string view's length in order to not overrun it. The reftable documentation explicitly notes that those two encoding schemas are supposed to be the same: Varint encoding ^^^^^^^^^^^^^^^ Varint encoding is identical to the ofs-delta encoding method used within pack files. Decoder works as follows: .... val = buf[ptr] & 0x7f while (buf[ptr] & 0x80) { ptr++ val = ((val + 1) << 7) | (buf[ptr] & 0x7f) } .... While at it, refactor `put_var_int()` in the same way by copying over the implementation of `encode_varint()`. While `put_var_int()` doesn't have an issue with overflows, it generates warnings with -Wsign-compare. The implementation of `encode_varint()` doesn't, is battle-tested and at the same time way simpler than what we currently have. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:

committed by
Junio C Hamano

parent
a204f92d1c
commit
072e3aa3a5
@ -58,6 +58,22 @@ static void t_varint_roundtrip(void)
|
||||
}
|
||||
}
|
||||
|
||||
static void t_varint_overflow(void)
|
||||
{
|
||||
unsigned char buf[] = {
|
||||
0xFF, 0xFF, 0xFF, 0xFF,
|
||||
0xFF, 0xFF, 0xFF, 0xFF,
|
||||
0xFF, 0x00,
|
||||
};
|
||||
struct string_view view = {
|
||||
.buf = buf,
|
||||
.len = sizeof(buf),
|
||||
};
|
||||
uint64_t value;
|
||||
int err = get_var_int(&value, &view);
|
||||
check_int(err, ==, -1);
|
||||
}
|
||||
|
||||
static void set_hash(uint8_t *h, int j)
|
||||
{
|
||||
for (int i = 0; i < hash_size(REFTABLE_HASH_SHA1); i++)
|
||||
@ -544,6 +560,7 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
|
||||
TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
|
||||
TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
|
||||
TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
|
||||
TEST(t_varint_overflow(), "get_var_int notices an integer overflow");
|
||||
TEST(t_key_roundtrip(), "reftable_encode_key and reftable_decode_key work");
|
||||
TEST(t_reftable_obj_record_roundtrip(), "record operations work on obj record");
|
||||
TEST(t_reftable_index_record_roundtrip(), "record operations work on index record");
|
||||
|
Reference in New Issue
Block a user