reftable/record: don't abort when decoding invalid ref value type

When decoding a ref record we read its value type from the block. In
case the type itself is invalid we call `abort()`. This is rather
heavy-handed though: the data we're reading is untrusted, so we should
treat the issue as a normal and not as a programming error.

Fix this by handling the error gracefully. Note that this also requires
us to set the value type later, as otherwise we might store an invalid
type in the record.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/reftable/record.c b/reftable/record.c
index fcd387ba5d57a3c912dc88a85c719a5195d8b41e..1fce441930ebe5a82985a6da9164a9280f6c74cb 100644 (file)
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -388,7 +388,6 @@ static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
        r->refname[key.len] = 0;
 
        r->update_index = update_index;
-       r->value_type = val_type;
        switch (val_type) {
        case REFTABLE_REF_VAL1:
                if (in.len < hash_size) {
@@ -426,9 +425,10 @@ static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
        case REFTABLE_REF_DELETION:
                break;
        default:
-               abort();
-               break;
+               err = REFTABLE_FORMAT_ERROR;
+               goto done;
        }
+       r->value_type = val_type;
 
        return start.len - in.len;
 

diff --git a/t/unit-tests/u-reftable-record.c b/t/unit-tests/u-reftable-record.c
index 1bf2e170dc96a0463cd8828e07e2067b41870c75..9c95083ef429c840b9dfb50f9ed5cfddb2adb99d 100644 (file)
--- a/t/unit-tests/u-reftable-record.c
+++ b/t/unit-tests/u-reftable-record.c
@@ -11,6 +11,7 @@
 #include "reftable/basics.h"
 #include "reftable/constants.h"
 #include "reftable/record.h"
+#include "reftable/reftable-error.h"
 
 static void t_copy(struct reftable_record *rec)
 {
@@ -202,6 +203,29 @@ void test_reftable_record__ref_record_roundtrip(void)
        reftable_buf_release(&scratch);
 }
 
+void test_reftable_record__ref_record_decode_invalid_value_type(void)
+{
+       struct reftable_buf scratch = REFTABLE_BUF_INIT;
+       struct reftable_record out = {
+               .type = REFTABLE_BLOCK_TYPE_REF,
+       };
+       struct reftable_buf key = REFTABLE_BUF_INIT;
+       uint8_t buffer[1024] = { 0 };
+       struct string_view dest = {
+               .buf = buffer,
+               .len = sizeof(buffer),
+       };
+
+       cl_must_pass(reftable_buf_addstr(&key, "refs/heads/master"));
+       cl_assert_equal_i(reftable_record_decode(&out, key, REFTABLE_NR_REF_VALUETYPES,
+                                                dest, REFTABLE_HASH_SIZE_SHA1, &scratch),
+                         REFTABLE_FORMAT_ERROR);
+
+       reftable_record_release(&out);
+       reftable_buf_release(&key);
+       reftable_buf_release(&scratch);
+}
+
 void test_reftable_record__log_record_comparison(void)
 {
        struct reftable_record in[3] = {