Skip to content

Commit

Permalink
box: fix out of bound write in error_payload_destroy()
Browse files Browse the repository at this point in the history
If `strlen(name)` is 1, `value_size` is 1, and `extra` is 0, then 15 bytes
are allocated for `struct error_field` in error_payload_prepare(). However,
the size of this structure is 16 because of the padding for the alignment.
Thus TRASH() in error_payload_destroy() writes 1 byte beyond the structure.

Closes tarantool#9098

NO_DOC=bugfix
  • Loading branch information
Gumix committed Sep 7, 2023
1 parent 3421a3b commit d927d3a
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## bugfix/core

* Fixed a possible out-of-bound write on unpacking a MsgPack error extension
(gh-9098).
10 changes: 8 additions & 2 deletions src/lib/core/error_payload.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ error_payload_prepare(struct error_payload *e, const char *name,
uint32_t name_size = strlen(name) + 1;
uint32_t data_offset = offsetof(struct error_field, name[name_size]);
uint32_t total = data_offset + value_size + extra;
/*
* For small values of `name_size', `value_size' and `extra', the size
* of `struct error_field' can be greater than the calculated `total'
* because of structure padding.
*/
uint32_t field_size = MAX(sizeof(struct error_field), total);

struct error_field **fields = e->fields;
int count = e->count;
Expand All @@ -49,13 +55,13 @@ error_payload_prepare(struct error_payload *e, const char *name,
f = fields[i];
if (strcmp(name, f->name) != 0)
continue;
f = xrealloc(f, total);
f = xrealloc(f, field_size);
goto set;
}
e->count = ++count;
fields = xrealloc(fields, sizeof(fields[0]) * count);
e->fields = fields;
f = xmalloc(total);
f = xmalloc(field_size);
memcpy(f->name, name, name_size);
set:
f->data = (char *)f + data_offset;
Expand Down
45 changes: 44 additions & 1 deletion test/unit/xrow.cc
Original file line number Diff line number Diff line change
Expand Up @@ -678,13 +678,55 @@ test_xrow_decode_error_4(void)
footer();
}

static void
test_xrow_decode_error_9098(void)
{
header();
plan(1);

uint8_t data[] = {
0x81, /* MP_MAP of 1 element */
0x52, /* IPROTO_ERROR: */
0x81, /* MP_MAP of 1 element */
0x00, /* MP_ERROR_STACK: */
0x91, /* MP_ARRAY of 1 element */
0x84, /* MP_MAP of 4 elements */
0x00, 0xa1, 0x00, /* MP_ERROR_TYPE: "" */
0x01, 0xa1, 0x00, /* MP_ERROR_FILE: "" */
0x03, 0xa1, 0x00, /* MP_ERROR_MESSAGE: "" */
0x06, /* MP_ERROR_FIELDS: */
0x81, /* MP_MAP of 1 element */
0xa1, 0x78, 0x2a /* "x": 42 */
};

struct iovec body;
body.iov_base = (void *)data;
body.iov_len = sizeof(data);

struct xrow_header row;
row.type = IPROTO_TYPE_ERROR;
row.body[0] = body;
row.bodycnt = 1;

xrow_decode_error(&row);

uint64_t payload_value;
struct error *e = diag_last_error(diag_get());
error_get_uint(e, "x", &payload_value);
is(payload_value, 42, "decoded payload");
diag_destroy(diag_get());

check_plan();
footer();
}

int
main(void)
{
memory_init();
fiber_init(fiber_c_invoke);
header();
plan(10);
plan(11);

random_init();

Expand All @@ -699,6 +741,7 @@ main(void)
test_xrow_decode_error_2();
test_xrow_decode_error_3();
test_xrow_decode_error_4();
test_xrow_decode_error_9098();

random_free();
fiber_free();
Expand Down

0 comments on commit d927d3a

Please sign in to comment.