From: Mathieu Desnoyers Date: Wed, 27 Nov 2013 08:19:31 +0000 (-0500) Subject: Fix: add stricter checks on packet boundaries X-Git-Tag: v1.2.0-rc1~39^2~15 X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=70fd5a515016525961d4bef0d627616abdc9bc28 Fix: add stricter checks on packet boundaries Fixes #699 Signed-off-by: Mathieu Desnoyers --- diff --git a/converter/babeltrace-log.c b/converter/babeltrace-log.c index b6d798f7..5a8b26a8 100644 --- 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 ea3ecfd8..b1e8d6c5 100644 --- 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 68902836..afe5e8d2 100644 --- 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 257341ad..189943e6 100644 --- 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 898c3672..5dae7d02 100644 --- 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 a2433bf5..3dd1414a 100644 --- 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 106f682c..cbf53c1e 100644 --- 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 b3d6396a..fc7b7ad4 100644 --- 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 ad4fcb5a..c2ad4c24 100644 --- 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 84548527..fad578ff 100644 --- 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; }