Fix: add stricter checks on packet boundaries

Fixes #699

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/converter/babeltrace-log.c b/converter/babeltrace-log.c
index b6d798f797d0f86094b69c3e507dbf045ebed60f..5a8b26a8076ec4c26cdbdf3390403cf61b6f3e0f 100644 (file)
--- a/converter/babeltrace-log.c
+++ b/converter/babeltrace-log.c
@@ -124,23 +124,36 @@ void write_packet_header(struct ctf_stream_pos *pos, unsigned char *uuid)
 
        /* magic */
        ctf_dummy_pos(pos, &dummy);
-       ctf_align_pos(&dummy, sizeof(uint32_t) * CHAR_BIT);
-       ctf_move_pos(&dummy, sizeof(uint32_t) * CHAR_BIT);
+       if (!ctf_align_pos(&dummy, sizeof(uint32_t) * CHAR_BIT))
+               goto error;
+       if (!ctf_move_pos(&dummy, sizeof(uint32_t) * CHAR_BIT))
+               goto error;
        assert(!ctf_pos_packet(&dummy));
 
-       ctf_align_pos(pos, sizeof(uint32_t) * CHAR_BIT);
+       if (!ctf_align_pos(pos, sizeof(uint32_t) * CHAR_BIT))
+               goto error;
        *(uint32_t *) ctf_get_pos_addr(pos) = 0xC1FC1FC1;
-       ctf_move_pos(pos, sizeof(uint32_t) * CHAR_BIT);
+       if (!ctf_move_pos(pos, sizeof(uint32_t) * CHAR_BIT))
+               goto error;
 
        /* uuid */
        ctf_dummy_pos(pos, &dummy);
-       ctf_align_pos(&dummy, sizeof(uint8_t) * CHAR_BIT);
-       ctf_move_pos(&dummy, 16 * CHAR_BIT);
+       if (!ctf_align_pos(&dummy, sizeof(uint8_t) * CHAR_BIT))
+               goto error;
+       if (!ctf_move_pos(&dummy, 16 * CHAR_BIT))
+               goto error;
        assert(!ctf_pos_packet(&dummy));
 
-       ctf_align_pos(pos, sizeof(uint8_t) * CHAR_BIT);
+       if (!ctf_align_pos(pos, sizeof(uint8_t) * CHAR_BIT))
+               goto error;
        memcpy(ctf_get_pos_addr(pos), uuid, BABELTRACE_UUID_LEN);
-       ctf_move_pos(pos, BABELTRACE_UUID_LEN * CHAR_BIT);
+       if (!ctf_move_pos(pos, BABELTRACE_UUID_LEN * CHAR_BIT))
+               goto error;
+       return;
+
+error:
+       fprintf(stderr, "[error] Out of packet bounds when writing packet header\n");
+       abort();
 }
 
 static
@@ -150,24 +163,37 @@ void write_packet_context(struct ctf_stream_pos *pos)
 
        /* content_size */
        ctf_dummy_pos(pos, &dummy);
-       ctf_align_pos(&dummy, sizeof(uint64_t) * CHAR_BIT);
-       ctf_move_pos(&dummy, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_align_pos(&dummy, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
+       if (!ctf_move_pos(&dummy, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
        assert(!ctf_pos_packet(&dummy));
 
-       ctf_align_pos(pos, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_align_pos(pos, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
        *(uint64_t *) ctf_get_pos_addr(pos) = ~0ULL;    /* Not known yet */
        pos->content_size_loc = (uint64_t *) ctf_get_pos_addr(pos);
-       ctf_move_pos(pos, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_move_pos(pos, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
 
        /* packet_size */
        ctf_dummy_pos(pos, &dummy);
-       ctf_align_pos(&dummy, sizeof(uint64_t) * CHAR_BIT);
-       ctf_move_pos(&dummy, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_align_pos(&dummy, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
+       if (!ctf_move_pos(&dummy, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
        assert(!ctf_pos_packet(&dummy));
 
-       ctf_align_pos(pos, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_align_pos(pos, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
        *(uint64_t *) ctf_get_pos_addr(pos) = pos->packet_size;
-       ctf_move_pos(pos, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_move_pos(pos, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
+       return;
+
+error:
+       fprintf(stderr, "[error] Out of packet bounds when writing packet context\n");
+       abort();
 }
 
 static
@@ -225,10 +251,17 @@ void write_event_header(struct ctf_stream_pos *pos, char *line,
                }
        }
        /* timestamp */
-       ctf_align_pos(pos, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_align_pos(pos, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
        if (!pos->dummy)
                *(uint64_t *) ctf_get_pos_addr(pos) = *ts;
-       ctf_move_pos(pos, sizeof(uint64_t) * CHAR_BIT);
+       if (!ctf_move_pos(pos, sizeof(uint64_t) * CHAR_BIT))
+               goto error;
+       return;
+
+error:
+       fprintf(stderr, "[error] Out of packet bounds when writing event header\n");
+       abort();
 }
 
 static
@@ -245,8 +278,10 @@ void trace_string(char *line, struct ctf_stream_pos *pos, size_t len)
        for (;;) {
                ctf_dummy_pos(pos, &dummy);
                write_event_header(&dummy, line, &tline, len, &tlen, &ts);
-               ctf_align_pos(&dummy, sizeof(uint8_t) * CHAR_BIT);
-               ctf_move_pos(&dummy, tlen * CHAR_BIT);
+               if (!ctf_align_pos(&dummy, sizeof(uint8_t) * CHAR_BIT))
+                       goto error;
+               if (!ctf_move_pos(&dummy, tlen * CHAR_BIT))
+                       goto error;
                if (ctf_pos_packet(&dummy)) {
                        ctf_pos_pad_packet(pos);
                        write_packet_header(pos, s_uuid);
@@ -263,9 +298,16 @@ void trace_string(char *line, struct ctf_stream_pos *pos, size_t len)
        }
 
        write_event_header(pos, line, &tline, len, &tlen, &ts);
-       ctf_align_pos(pos, sizeof(uint8_t) * CHAR_BIT);
+       if (!ctf_align_pos(pos, sizeof(uint8_t) * CHAR_BIT))
+               goto error;
        memcpy(ctf_get_pos_addr(pos), tline, tlen);
-       ctf_move_pos(pos, tlen * CHAR_BIT);
+       if (!ctf_move_pos(pos, tlen * CHAR_BIT))
+               goto error;
+       return;
+
+error:
+       fprintf(stderr, "[error] Out of packet bounds when writing event payload\n");
+       abort();
 }
 
 static

diff --git a/formats/ctf/types/array.c b/formats/ctf/types/array.c
index ea3ecfd8a656d5db9b4f389ee1302b5b6b5926ca..b1e8d6c5af7b316110b31920a7a61ed125ac4c98 100644 (file)
--- a/formats/ctf/types/array.c
+++ b/formats/ctf/types/array.c
@@ -48,7 +48,8 @@ int ctf_array_read(struct bt_stream_pos *ppos, struct bt_definition *definition)
                        if (integer_declaration->len == CHAR_BIT
                            && integer_declaration->p.alignment == CHAR_BIT) {
 
-                               ctf_align_pos(pos, integer_declaration->p.alignment);
+                               if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+                                       return -EFAULT;
                                if (!ctf_pos_access_ok(pos, array_declaration->len * CHAR_BIT))
                                        return -EFAULT;
 
@@ -56,7 +57,8 @@ int ctf_array_read(struct bt_stream_pos *ppos, struct bt_definition *definition)
                                g_string_insert_len(array_definition->string,
                                        0, (char *) ctf_get_pos_addr(pos),
                                        array_declaration->len);
-                               ctf_move_pos(pos, array_declaration->len * CHAR_BIT);
+                               if (!ctf_move_pos(pos, array_declaration->len * CHAR_BIT))
+                                       return -EFAULT;
                                return 0;
                        }
                }
@@ -84,14 +86,16 @@ int ctf_array_write(struct bt_stream_pos *ppos, struct bt_definition *definition
                        if (integer_declaration->len == CHAR_BIT
                            && integer_declaration->p.alignment == CHAR_BIT) {
 
-                               ctf_align_pos(pos, integer_declaration->p.alignment);
+                               if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+                                       return -EFAULT;
                                if (!ctf_pos_access_ok(pos, array_declaration->len * CHAR_BIT))
                                        return -EFAULT;
 
                                memcpy((char *) ctf_get_pos_addr(pos),
                                        array_definition->string->str,
                                        array_declaration->len);
-                               ctf_move_pos(pos, array_declaration->len * CHAR_BIT);
+                               if (!ctf_move_pos(pos, array_declaration->len * CHAR_BIT))
+                                       return -EFAULT;
                                return 0;
                        }
                }

diff --git a/formats/ctf/types/float.c b/formats/ctf/types/float.c
index 689028360d4fe8f258d307e014e54103fb03201e..afe5e8d2cf2ef9d66a1cce2df2727bea70a4ba34 100644 (file)
--- a/formats/ctf/types/float.c
+++ b/formats/ctf/types/float.c
@@ -201,7 +201,10 @@ int ctf_float_read(struct bt_stream_pos *ppos, struct bt_definition *definition)
        mmap_align_set_addr(&mma, (char *) u.bits);
        destp.base_mma = &mma;
        destp.packet_size = sizeof(u) * CHAR_BIT;
-       ctf_align_pos(pos, float_declaration->p.alignment);
+       if (!ctf_align_pos(pos, float_declaration->p.alignment)) {
+               ret = -EFAULT;
+               goto end_unref;
+       }
        ret = _ctf_float_copy(&destp.parent, tmpfloat, ppos, float_definition);
        switch (float_declaration->mantissa->len + 1) {
        case FLT_MANT_DIG:
@@ -268,7 +271,10 @@ int ctf_float_write(struct bt_stream_pos *ppos, struct bt_definition *definition
                ret = -EINVAL;
                goto end_unref;
        }
-       ctf_align_pos(pos, float_declaration->p.alignment);
+       if (!ctf_align_pos(pos, float_declaration->p.alignment)) {
+               ret = -EFAULT;
+               goto end_unref;
+       }
        ret = _ctf_float_copy(ppos, float_definition, &srcp.parent, tmpfloat);
 
 end_unref:

diff --git a/formats/ctf/types/integer.c b/formats/ctf/types/integer.c
index 257341adfa41ce6320a68a00e04cf6bc4d43490f..189943e6052438bc4b6e3cb2895dcbfc54005562 100644 (file)
--- a/formats/ctf/types/integer.c
+++ b/formats/ctf/types/integer.c
@@ -49,7 +49,8 @@ int _aligned_integer_read(struct bt_stream_pos *ppos,
        struct ctf_stream_pos *pos = ctf_pos(ppos);
        int rbo = (integer_declaration->byte_order != BYTE_ORDER);      /* reverse byte order */
 
-       ctf_align_pos(pos, integer_declaration->p.alignment);
+       if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+               return -EFAULT;
 
        if (!ctf_pos_access_ok(pos, integer_declaration->len))
                return -EFAULT;
@@ -136,7 +137,8 @@ int _aligned_integer_read(struct bt_stream_pos *ppos,
                        assert(0);
                }
        }
-       ctf_move_pos(pos, integer_declaration->len);
+       if (!ctf_move_pos(pos, integer_declaration->len))
+               return -EFAULT;
        return 0;
 }
 
@@ -151,7 +153,8 @@ int _aligned_integer_write(struct bt_stream_pos *ppos,
        struct ctf_stream_pos *pos = ctf_pos(ppos);
        int rbo = (integer_declaration->byte_order != BYTE_ORDER);      /* reverse byte order */
 
-       ctf_align_pos(pos, integer_declaration->p.alignment);
+       if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+               return -EFAULT;
 
        if (!ctf_pos_access_ok(pos, integer_declaration->len))
                return -EFAULT;
@@ -207,7 +210,8 @@ int _aligned_integer_write(struct bt_stream_pos *ppos,
                }
        }
 end:
-       ctf_move_pos(pos, integer_declaration->len);
+       if (!ctf_move_pos(pos, integer_declaration->len))
+               return -EFAULT;
        return 0;
 }
 
@@ -224,7 +228,8 @@ int ctf_integer_read(struct bt_stream_pos *ppos, struct bt_definition *definitio
                return _aligned_integer_read(ppos, definition);
        }
 
-       ctf_align_pos(pos, integer_declaration->p.alignment);
+       if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+               return -EFAULT;
 
        if (!ctf_pos_access_ok(pos, integer_declaration->len))
                return -EFAULT;
@@ -252,7 +257,8 @@ int ctf_integer_read(struct bt_stream_pos *ppos, struct bt_definition *definitio
                                pos->offset, integer_declaration->len,
                                &integer_definition->value._signed);
        }
-       ctf_move_pos(pos, integer_declaration->len);
+       if (!ctf_move_pos(pos, integer_declaration->len))
+               return -EFAULT;
        return 0;
 }
 
@@ -269,7 +275,8 @@ int ctf_integer_write(struct bt_stream_pos *ppos, struct bt_definition *definiti
                return _aligned_integer_write(ppos, definition);
        }
 
-       ctf_align_pos(pos, integer_declaration->p.alignment);
+       if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+               return -EFAULT;
 
        if (!ctf_pos_access_ok(pos, integer_declaration->len))
                return -EFAULT;
@@ -300,6 +307,7 @@ int ctf_integer_write(struct bt_stream_pos *ppos, struct bt_definition *definiti
                                integer_definition->value._signed);
        }
 end:
-       ctf_move_pos(pos, integer_declaration->len);
+       if (!ctf_move_pos(pos, integer_declaration->len))
+               return -EFAULT;
        return 0;
 }

diff --git a/formats/ctf/types/sequence.c b/formats/ctf/types/sequence.c
index 898c36725672c8e4b01084e1850e4c27050634d6..5dae7d0281d4610c586d00b2ee487e8ddb5a063d 100644 (file)
--- a/formats/ctf/types/sequence.c
+++ b/formats/ctf/types/sequence.c
@@ -48,14 +48,16 @@ int ctf_sequence_read(struct bt_stream_pos *ppos, struct bt_definition *definiti
                            && integer_declaration->p.alignment == CHAR_BIT) {
                                uint64_t len = bt_sequence_len(sequence_definition);
 
-                               ctf_align_pos(pos, integer_declaration->p.alignment);
+                               if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+                                       return -EFAULT;
                                if (!ctf_pos_access_ok(pos, len * CHAR_BIT))
                                        return -EFAULT;
 
                                g_string_assign(sequence_definition->string, "");
                                g_string_insert_len(sequence_definition->string,
                                        0, (char *) ctf_get_pos_addr(pos), len);
-                               ctf_move_pos(pos, len * CHAR_BIT);
+                               if (!ctf_move_pos(pos, len * CHAR_BIT))
+                                       return -EFAULT;
                                return 0;
                        }
                }
@@ -83,13 +85,15 @@ int ctf_sequence_write(struct bt_stream_pos *ppos, struct bt_definition *definit
                            && integer_declaration->p.alignment == CHAR_BIT) {
                                uint64_t len = bt_sequence_len(sequence_definition);
 
-                               ctf_align_pos(pos, integer_declaration->p.alignment);
+                               if (!ctf_align_pos(pos, integer_declaration->p.alignment))
+                                       return -EFAULT;
                                if (!ctf_pos_access_ok(pos, len * CHAR_BIT))
                                        return -EFAULT;
 
                                memcpy((char *) ctf_get_pos_addr(pos),
                                        sequence_definition->string->str, len);
-                               ctf_move_pos(pos, len * CHAR_BIT);
+                               if (!ctf_move_pos(pos, len * CHAR_BIT))
+                                       return -EFAULT;
                                return 0;
                        }
                }

diff --git a/formats/ctf/types/string.c b/formats/ctf/types/string.c
index a2433bf5922d2013e80f19a1e1897842657de535..3dd1414a0e078882160c7de6d28226d7a27b0973 100644 (file)
--- a/formats/ctf/types/string.c
+++ b/formats/ctf/types/string.c
@@ -42,7 +42,8 @@ int ctf_string_read(struct bt_stream_pos *ppos, struct bt_definition *definition
        ssize_t max_len;
        char *srcaddr;
 
-       ctf_align_pos(pos, string_declaration->p.alignment);
+       if (!ctf_align_pos(pos, string_declaration->p.alignment))
+               return -EFAULT;
 
        srcaddr = ctf_get_pos_addr(pos);
        if (pos->offset == EOF)
@@ -64,7 +65,8 @@ int ctf_string_read(struct bt_stream_pos *ppos, struct bt_definition *definition
        printf_debug("CTF string read %s\n", srcaddr);
        memcpy(string_definition->value, srcaddr, len);
        string_definition->len = len;
-       ctf_move_pos(pos, len * CHAR_BIT);
+       if (!ctf_move_pos(pos, len * CHAR_BIT))
+               return -EFAULT;
        return 0;
 }
 
@@ -79,7 +81,8 @@ int ctf_string_write(struct bt_stream_pos *ppos,
        size_t len;
        char *destaddr;
 
-       ctf_align_pos(pos, string_declaration->p.alignment);
+       if (!ctf_align_pos(pos, string_declaration->p.alignment))
+               return -EFAULT;
        assert(string_definition->value != NULL);
        len = string_definition->len;
 
@@ -91,6 +94,7 @@ int ctf_string_write(struct bt_stream_pos *ppos,
        destaddr = ctf_get_pos_addr(pos);
        memcpy(destaddr, string_definition->value, len);
 end:
-       ctf_move_pos(pos, len * CHAR_BIT);
+       if (!ctf_move_pos(pos, len * CHAR_BIT))
+               return -EFAULT;
        return 0;
 }

diff --git a/formats/ctf/types/struct.c b/formats/ctf/types/struct.c
index 106f682c8ee2d886a9daded1bb32d027e20a7abc..cbf53c1edb58a9d29a03ff449a7c7d16b8401d14 100644 (file)
--- a/formats/ctf/types/struct.c
+++ b/formats/ctf/types/struct.c
@@ -33,6 +33,7 @@ int ctf_struct_rw(struct bt_stream_pos *ppos, struct bt_definition *definition)
        struct bt_declaration *declaration = definition->declaration;
        struct ctf_stream_pos *pos = ctf_pos(ppos);
 
-       ctf_align_pos(pos, declaration->alignment);
+       if (!ctf_align_pos(pos, declaration->alignment))
+               return -EFAULT;
        return bt_struct_rw(ppos, definition);
 }

diff --git a/formats/ctf/types/variant.c b/formats/ctf/types/variant.c
index b3d6396a6f9f075f21be4fd71e871a55c2c038f6..fc7b7ad449d935ea4dcf9e32afda839463250141 100644 (file)
--- a/formats/ctf/types/variant.c
+++ b/formats/ctf/types/variant.c
@@ -33,6 +33,7 @@ int ctf_variant_rw(struct bt_stream_pos *ppos, struct bt_definition *definition)
        struct bt_declaration *declaration = definition->declaration;
        struct ctf_stream_pos *pos = ctf_pos(ppos);
 
-       ctf_align_pos(pos, declaration->alignment);
+       if (!ctf_align_pos(pos, declaration->alignment))
+               return -EFAULT;
        return bt_variant_rw(ppos, definition);
 }

diff --git a/formats/ctf/writer/event-fields.c b/formats/ctf/writer/event-fields.c
index ad4fcb5a06c1ed5d3ad4e5636fdfab603f832623..c2ad4c2447537b2370823b21f5dee2f238e53f58 100644 (file)
--- a/formats/ctf/writer/event-fields.c
+++ b/formats/ctf/writer/event-fields.c
@@ -1131,7 +1131,10 @@ int bt_ctf_field_structure_serialize(struct bt_ctf_field *field,
                }
        }
 
-       ctf_align_pos(pos, field->type->declaration->alignment);
+       if (!ctf_align_pos(pos, field->type->declaration->alignment)) {
+               ret = -1;
+               goto end;
+       }
 
        for (i = 0; i < structure->fields->len; i++) {
                struct bt_ctf_field *field = g_ptr_array_index(

diff --git a/include/babeltrace/ctf/types.h b/include/babeltrace/ctf/types.h
index 84548527eb0400a4d603673dcd49f97876e3aa07..fad578ff267d3f88b2a966b9c60f6aeacb334c21 100644 (file)
--- a/include/babeltrace/ctf/types.h
+++ b/include/babeltrace/ctf/types.h
@@ -129,28 +129,41 @@ int ctf_fini_pos(struct ctf_stream_pos *pos);
 /*
  * move_pos - move position of a relative bit offset
  *
+ * Return 1 if OK, 0 if out-of-bound.
+ *
  * TODO: allow larger files by updating base too.
  */
 static inline
-void ctf_move_pos(struct ctf_stream_pos *pos, uint64_t bit_offset)
+int ctf_move_pos(struct ctf_stream_pos *pos, uint64_t bit_offset)
 {
+       uint64_t max_len;
+
        printf_debug("ctf_move_pos test EOF: %" PRId64 "\n", pos->offset);
        if (unlikely(pos->offset == EOF))
-               return;
+               return 0;
+       if (pos->flags & PROT_READ)
+               max_len = pos->content_size;
+       else
+               max_len = pos->packet_size;
+       if (unlikely(pos->offset + bit_offset > max_len))
+               return 0;
 
        pos->offset += bit_offset;
        printf_debug("ctf_move_pos after increment: %" PRId64 "\n", pos->offset);
+       return 1;
 }
 
 /*
  * align_pos - align position on a bit offset (> 0)
  *
+ * Return 1 if OK, 0 if out-of-bound.
+ *
  * TODO: allow larger files by updating base too.
  */
 static inline
-void ctf_align_pos(struct ctf_stream_pos *pos, uint64_t bit_offset)
+int ctf_align_pos(struct ctf_stream_pos *pos, uint64_t bit_offset)
 {
-       ctf_move_pos(pos, offset_align(pos->offset, bit_offset));
+       return ctf_move_pos(pos, offset_align(pos->offset, bit_offset));
 }
 
 static inline
@@ -191,9 +204,15 @@ void ctf_pos_pad_packet(struct ctf_stream_pos *pos)
 static inline
 int ctf_pos_access_ok(struct ctf_stream_pos *pos, uint64_t bit_len)
 {
+       uint64_t max_len;
+
        if (unlikely(pos->offset == EOF))
                return 0;
-       if (unlikely(pos->offset + bit_len > pos->packet_size))
+       if (pos->flags & PROT_READ)
+               max_len = pos->content_size;
+       else
+               max_len = pos->packet_size;
+       if (unlikely(pos->offset + bit_len > max_len))
                return 0;
        return 1;
 }