osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

tvb: gracefully handle reading 0 bytes from an empty buffer 

proto_tree_add_item with a zero length argument could end up calling
tvb_get_ptr to retrieve the (empty) backing buffer. This empty tvb was
possibly the result of bad reassembly, but let's gracefully handle it to
avoid a dissector exception.

Call trace for the original exception (only present on the first pass):

    proto_report_dissector_bug (format=0x7ffffffecea0 "") at epan/proto.c:1368
    ensure_contiguous_no_exception (tvb=0x6060001a5460, offset=0, length=0, pexception=0x7ffffffed060) at epan/tvbuff.c:775
    ensure_contiguous (tvb=0x6060001a5460, offset=0, length=0) at epan/tvbuff.c:785
    tvb_get_ptr (tvb=0x6060001a5460, offset=0, length=0) at epan/tvbuff.c:906
    subset_get_ptr (tvb=0x607000194b90, abs_offset=0, abs_length=0) at epan/tvbuff_subset.c:58
    ensure_contiguous_no_exception (tvb=0x607000194b90, offset=0, length=0, pexception=0x7ffffffed3c0) at epan/tvbuff.c:773
    ensure_contiguous (tvb=0x607000194b90, offset=0, length=0) at epan/tvbuff.c:785
    tvb_get_ptr (tvb=0x607000194b90, offset=0, length=0) at epan/tvbuff.c:906
    proto_tree_set_bytes_tvb (fi=0x608000535ca0, tvb=0x607000194b90, offset=0, length=0) at epan/proto.c:3862
    proto_tree_new_item (new_fi=0x608000535ca0, tree=0x604000543150, tvb=0x607000194b90, start=0, length=0, encoding=0) at epan/proto.c:2318
    proto_tree_add_item_new (tree=0x604000543150, hfinfo=0x7ffff30e91f8, tvb=0x607000194b90, start=0, length=0, encoding=0) at epan/proto.c:3381
    proto_tree_add_item (tree=0x604000543150, hfindex=65120, tvb=0x607000194b90, start=0, length=0, encoding=0) at epan/proto.c:3391
    dissect_body_data (tree=0x604000543150, pinfo=0x614000000a58, tvb=0x607000194b90, start=0, length=0, encoding=0) at epan/dissectors/packet-http2.c:1974

Change-Id: Icfae83d61ddcc9e26f16eab7f6e0e84e2f0d73ac
Reviewed-on: https://code.wireshark.org/review/29851
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
Peter Wu 2018-09-26 12:26:21 +02:00 committed by Anders Broman
parent 123bcb0362
commit 281dd22da9
2 changed files with 27 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
epan/tvbtest.c
View File

@ -95,7 +95,7 @@ test(tvbuff_t *tvb, const gchar* name,
ex_thrown = TRUE;
}
CATCH_ALL {
printf("02: Caught wrong exception: %lu\n", exc->except_id.except_code);
printf("03: Caught wrong exception: %lu\n", exc->except_id.except_code);
}
ENDTRY;
@ -121,7 +121,7 @@ test(tvbuff_t *tvb, const gchar* name,
printf("04: Caught wrong exception: ReportedBoundsError\n");
}
CATCH_ALL {
printf("02: Caught wrong exception: %lu\n", exc->except_id.except_code);
printf("04: Caught wrong exception: %lu\n", exc->except_id.except_code);
}
ENDTRY;
@ -135,7 +135,7 @@ test(tvbuff_t *tvb, const gchar* name,
/* Test boundary case. A BoundsError exception should not be thrown. */
ex_thrown = FALSE;
TRY {
tvb_get_ptr(tvb, 0, 1);
tvb_get_ptr(tvb, 0, length ? 1 : 0);
}
CATCH(BoundsError) {
ex_thrown = TRUE;
@ -147,7 +147,7 @@ test(tvbuff_t *tvb, const gchar* name,
printf("05: Caught wrong exception: ReportedBoundsError\n");
}
CATCH_ALL {
printf("02: Caught wrong exception: %lu\n", exc->except_id.except_code);
printf("05: Caught wrong exception: %lu\n", exc->except_id.except_code);
}
ENDTRY;
@ -161,7 +161,7 @@ test(tvbuff_t *tvb, const gchar* name,
/* Test boundary case. A BoundsError exception should not be thrown. */
ex_thrown = FALSE;
TRY {
tvb_get_ptr(tvb, -1, 1);
tvb_get_ptr(tvb, -1, length ? 1 : 0);
}
CATCH(BoundsError) {
ex_thrown = TRUE;
@ -173,7 +173,7 @@ test(tvbuff_t *tvb, const gchar* name,
printf("06: Caught wrong exception: ReportedBoundsError\n");
}
CATCH_ALL {
printf("02: Caught wrong exception: %lu\n", exc->except_id.except_code);
printf("06: Caught wrong exception: %lu\n", exc->except_id.except_code);
}
ENDTRY;
@ -258,7 +258,8 @@ test(tvbuff_t *tvb, const gchar* name,
/* One big memdup */
ptr = (guint8*)tvb_memdup(NULL, tvb, 0, -1);
if (memcmp(ptr, expected_data, length) != 0) {
if ((length != 0 && memcmp(ptr, expected_data, length) != 0) ||
(length == 0 && ptr != NULL)) {
printf("12: Failed TVB=%s Offset=0 Length=-1 "
"Bad memdup\n", name);
failed = TRUE;
@ -279,9 +280,11 @@ run_tests(void)
int i, j;
tvbuff_t *tvb_parent;
tvbuff_t *tvb_empty;
tvbuff_t *tvb_small[3];
tvbuff_t *tvb_large[3];
tvbuff_t *tvb_subset[6];
tvbuff_t *tvb_empty_subset;
guint8 *small[3];
guint small_length[3];
guint small_reported_length[3];
@ -326,6 +329,10 @@ run_tests(void)
tvb_set_free_cb(tvb_large[i], g_free);
}
/* Test empty tvb */
tvb_empty = tvb_new_child_real_data(tvb_parent, NULL, 0, 1);
test(tvb_empty, "Empty", NULL, 0, 1);
/* Test the "real" tvbuff objects. */
test(tvb_small[0], "Small 0", small[0], small_length[0], small_reported_length[0]);
test(tvb_small[1], "Small 1", small[1], small_length[1], small_reported_length[1]);
@ -373,6 +380,10 @@ run_tests(void)
test(tvb_subset[4], "Subset 4", subset[4], subset_length[4], subset_reported_length[4]);
test(tvb_subset[5], "Subset 5", subset[5], subset_length[5], subset_reported_length[5]);
/* Subset of an empty tvb. */
tvb_empty_subset = tvb_new_subset_length_caplen(tvb_empty, 0, 0, 1);
test(tvb_empty_subset, "Empty Subset", NULL, 0, 1);
/* One Real */
printf("Making Composite 0\n");
tvb_comp[0] = tvb_new_composite();

10
epan/tvbuff.c
View File

@ -762,6 +762,14 @@ ensure_contiguous_no_exception(tvbuff_t *tvb, const gint offset, const gint leng
return NULL;
}
/*
* Special case: if the caller (e.g. tvb_get_ptr) requested no data,
* then it is acceptable to have an empty tvb (!tvb->real_data).
*/
if (length == 0) {
return NULL;
}
/*
* We know that all the data is present in the tvbuff, so
* no exceptions should be thrown.
@ -783,7 +791,7 @@ ensure_contiguous(tvbuff_t *tvb, const gint offset, const gint length)
const guint8 *p;
p = ensure_contiguous_no_exception(tvb, offset, length, &exception);
if (p == NULL) {
if (p == NULL && length != 0) {
DISSECTOR_ASSERT(exception > 0);
THROW(exception);
}