tests: Improve validity of binary GDBusMessage parsing tests

These tests were originally written using the output directly from a
fuzzer which had triggered the bugs we’re testing for. However, that
means they’re liable to no longer test what they’re intended to test if
the `GDBusMessage` parsing code is changed to (for example) check for
certain errors earlier in future.

It’s better to only have one invalidity in each binary blob, so change
the test messages to all be valid apart from the specific thing they’re
testing for.

The changes were based on reading the D-Bus specification directly:
https://dbus.freedesktop.org/doc/dbus-specification.html

During these changes I found one problem in
`test_message_parse_deep_header_nesting()` where it wasn’t actually
nesting variants in the header deeply enough to trigger the bug it was
supposed to be testing for. Fixed that.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #1963
This commit is contained in:
Philip Withnall 2020-11-18 12:49:54 +00:00
parent fe5217bc2b
commit f936bba0d1

View File

@ -1118,28 +1118,37 @@ test_message_serialize_double_array (void)
/* Test that an invalid header in a D-Bus message (specifically, with a type /* Test that an invalid header in a D-Bus message (specifically, with a type
* which doesnt match whats expected for the given header) is gracefully * which doesnt match whats expected for the given header) is gracefully
* handled with an error rather than a crash. * handled with an error rather than a crash. */
* The set of bytes here come directly from fuzzer output. */
static void static void
test_message_parse_non_signature_header (void) test_message_parse_non_signature_header (void)
{ {
const guint8 data[] = { const guint8 data[] = {
'l', /* little-endian byte order */ 'l', /* little-endian byte order */
0x04, /* message type */ 0x02, /* message type (method return) */
0x0f, /* message flags */ 0x00, /* message flags (none) */
0x01, /* major protocol version */ 0x01, /* major protocol version */
0x00, 0x00, 0x00, 0x00, /* body length */ 0x00, 0x00, 0x00, 0x00, /* body length (in bytes) */
0x00, 0x00, 0x00, 0xbc, /* message serial */ 0x00, 0x00, 0x00, 0xbc, /* message serial */
/* a{yv} of header fields: /* a{yv} of header fields:
* (things start to be invalid below here) */ * (things start to be invalid below here) */
0x02, 0x00, 0x00, 0x00, /* array length (in bytes) */ 0x10, 0x00, 0x00, 0x00, /* array length (in bytes), must be a multiple of 8 */
G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE, /* array key */ 0x08, /* array key (SIGNATURE) */
/* Variant array value: */ /* Variant array value: */
0x04, /* signature length */ 0x04, /* signature length */
'd', 0x00, 0x00, 'F', /* signature (invalid) */ 'd', 0x00, 0x00, 'F', /* signature (invalid) */
0x00, /* nul terminator */ 0x00, /* nul terminator */
/* (Variant array value payload missing) */ /* (Variant array value payload missing) */
/* (message body length missing) */ /* alignment padding before the next header array element, as structs must
* be 8-aligned: */
0x00,
0x05, /* array key (REPLY_SERIAL, required for method return messages) */
/* Variant array value: */
0x01, /* signature length */
'u', /* one complete type */
0x00, /* nul terminator */
/* (Variant array value payload) */
0x00, 0x01, 0x02, 0x03,
/* (message body is zero-length) */
}; };
gsize size = sizeof (data); gsize size = sizeof (data);
GDBusMessage *message = NULL; GDBusMessage *message = NULL;
@ -1158,27 +1167,36 @@ test_message_parse_non_signature_header (void)
/* Test that an invalid header in a D-Bus message (specifically, containing a /* Test that an invalid header in a D-Bus message (specifically, containing a
* variant with an empty type signature) is gracefully handled with an error * variant with an empty type signature) is gracefully handled with an error
* rather than a crash. The set of bytes here come directly from fuzzer * rather than a crash. */
* output. */
static void static void
test_message_parse_empty_signature_header (void) test_message_parse_empty_signature_header (void)
{ {
const guint8 data[] = { const guint8 data[] = {
'l', /* little-endian byte order */ 'l', /* little-endian byte order */
0x20, /* message type */ 0x02, /* message type (method return) */
0x20, /* message flags */ 0x00, /* message flags (none) */
0x01, /* major protocol version */ 0x01, /* major protocol version */
0x20, 0x20, 0x20, 0x00, /* body length (invalid) */ 0x00, 0x00, 0x00, 0x00, /* body length (in bytes) */
0x20, 0x20, 0x20, 0x20, /* message serial */ 0x20, 0x20, 0x20, 0x20, /* message serial */
/* a{yv} of header fields: /* a{yv} of header fields:
* (things start to be even more invalid below here) */ * (things start to be invalid below here) */
0x20, 0x20, 0x20, 0x00, /* array length (in bytes) */ 0x10, 0x00, 0x00, 0x00, /* array length (in bytes), must be a multiple of 8 */
0x20, /* array key */ 0x20, /* array key (this is not currently a valid header field) */
/* Variant array value: */ /* Variant array value: */
0x00, /* signature length */ 0x00, /* signature length */
0x00, /* nul terminator */ 0x00, /* nul terminator */
/* (Variant array value payload missing) */ /* (Variant array value payload missing) */
/* (message body length missing) */ /* alignment padding before the next header array element, as structs must
* be 8-aligned: */
0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x05, /* array key (REPLY_SERIAL, required for method return messages) */
/* Variant array value: */
0x01, /* signature length */
'u', /* one complete type */
0x00, /* nul terminator */
/* (Variant array value payload) */
0x00, 0x01, 0x02, 0x03,
/* (message body is zero-length) */
}; };
gsize size = sizeof (data); gsize size = sizeof (data);
GDBusMessage *message = NULL; GDBusMessage *message = NULL;
@ -1197,28 +1215,37 @@ test_message_parse_empty_signature_header (void)
/* Test that an invalid header in a D-Bus message (specifically, containing a /* Test that an invalid header in a D-Bus message (specifically, containing a
* variant with a type signature containing multiple complete types) is * variant with a type signature containing multiple complete types) is
* gracefully handled with an error rather than a crash. The set of bytes here * gracefully handled with an error rather than a crash. */
* come directly from fuzzer output. */
static void static void
test_message_parse_multiple_signature_header (void) test_message_parse_multiple_signature_header (void)
{ {
const guint8 data[] = { const guint8 data[] = {
'l', /* little-endian byte order */ 'l', /* little-endian byte order */
0x20, /* message type */ 0x02, /* message type (method return) */
0x20, /* message flags */ 0x00, /* message flags (none) */
0x01, /* major protocol version */ 0x01, /* major protocol version */
0x20, 0x20, 0x20, 0x00, /* body length (invalid) */ 0x00, 0x00, 0x00, 0x00, /* body length (in bytes) */
0x20, 0x20, 0x20, 0x20, /* message serial */ 0x20, 0x20, 0x20, 0x20, /* message serial */
/* a{yv} of header fields: /* a{yv} of header fields:
* (things start to be even more invalid below here) */ * (things start to be invalid below here) */
0x20, 0x20, 0x20, 0x00, /* array length (in bytes) */ 0x10, 0x00, 0x00, 0x00, /* array length (in bytes), must be a multiple of 8 */
0x20, /* array key */ 0x20, /* array key (this is not currently a valid header field) */
/* Variant array value: */ /* Variant array value: */
0x02, /* signature length */ 0x02, /* signature length */
'b', 'b', /* two complete types */ 'b', 'b', /* two complete types */
0x00, /* nul terminator */ 0x00, /* nul terminator */
/* (Variant array value payload missing) */ /* (Variant array value payload missing) */
/* (message body length missing) */ /* alignment padding before the next header array element, as structs must
* be 8-aligned: */
0x00, 0x00, 0x00,
0x05, /* array key (REPLY_SERIAL, required for method return messages) */
/* Variant array value: */
0x01, /* signature length */
'u', /* one complete type */
0x00, /* nul terminator */
/* (Variant array value payload) */
0x00, 0x01, 0x02, 0x03,
/* (message body is zero-length) */
}; };
gsize size = sizeof (data); gsize size = sizeof (data);
GDBusMessage *message = NULL; GDBusMessage *message = NULL;
@ -1238,22 +1265,21 @@ test_message_parse_multiple_signature_header (void)
/* Test that an invalid header in a D-Bus message (specifically, containing a /* Test that an invalid header in a D-Bus message (specifically, containing a
* variant with a valid type signature that is too long to be a valid * variant with a valid type signature that is too long to be a valid
* #GVariantType due to exceeding the array nesting limits) is gracefully * #GVariantType due to exceeding the array nesting limits) is gracefully
* handled with an error rather than a crash. The set of bytes here come * handled with an error rather than a crash. */
* directly from fuzzer output. */
static void static void
test_message_parse_over_long_signature_header (void) test_message_parse_over_long_signature_header (void)
{ {
const guint8 data[] = { const guint8 data[] = {
'l', /* little-endian byte order */ 'l', /* little-endian byte order */
0x20, /* message type */ 0x02, /* message type (method return) */
0x20, /* message flags */ 0x00, /* message flags (none) */
0x01, /* major protocol version */ 0x01, /* major protocol version */
0x20, 0x20, 0x20, 0x01, /* body length (invalid) */ 0x00, 0x00, 0x00, 0x00, /* body length (in bytes) */
0x20, 0x20, 0x20, 0x20, /* message serial */ 0x20, 0x20, 0x20, 0x20, /* message serial */
/* a{yv} of header fields: /* a{yv} of header fields:
* (things start to be even more invalid below here) */ * (things start to be invalid below here) */
0x20, 0x00, 0x00, 0x00, /* array length (in bytes) */ 0xa0, 0x00, 0x00, 0x00, /* array length (in bytes), must be a multiple of 8 */
0x08, /* array key */ 0x08, /* array key (SIGNATURE) */
/* Variant array value: */ /* Variant array value: */
0x04, /* signature length */ 0x04, /* signature length */
'g', 0x00, 0x20, 0x20, /* one complete type plus some rubbish */ 'g', 0x00, 0x20, 0x20, /* one complete type plus some rubbish */
@ -1275,8 +1301,16 @@ test_message_parse_over_long_signature_header (void)
'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a',
'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a',
'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a',
'v' 'v',
/* (message body length missing) */ /* first header length is a multiple of 8 so no padding is needed */
0x05, /* array key (REPLY_SERIAL, required for method return messages) */
/* Variant array value: */
0x01, /* signature length */
'u', /* one complete type */
0x00, /* nul terminator */
/* (Variant array value payload) */
0x00, 0x01, 0x02, 0x03,
/* (message body is zero-length) */
}; };
gsize size = sizeof (data); gsize size = sizeof (data);
GDBusMessage *message = NULL; GDBusMessage *message = NULL;
@ -1295,20 +1329,20 @@ test_message_parse_over_long_signature_header (void)
/* Test that an invalid header in a D-Bus message (specifically, containing too /* Test that an invalid header in a D-Bus message (specifically, containing too
* many levels of nested variant) is gracefully handled with an error rather * many levels of nested variant) is gracefully handled with an error rather
* than a crash. The set of bytes here come almost directly from fuzzer output. */ * than a crash. */
static void static void
test_message_parse_deep_header_nesting (void) test_message_parse_deep_header_nesting (void)
{ {
const guint8 data[] = { const guint8 data[] = {
'l', /* little-endian byte order */ 'l', /* little-endian byte order */
0x20, /* message type */ 0x02, /* message type (method return) */
0x20, /* message flags */ 0x00, /* message flags (none) */
0x01, /* major protocol version */ 0x01, /* major protocol version */
0x20, 0x20, 0x20, 0x00, /* body length (invalid) */ 0x00, 0x00, 0x00, 0x00, /* body length (in bytes) */
0x20, 0x20, 0x20, 0x20, /* message serial */ 0x20, 0x20, 0x20, 0x20, /* message serial */
/* a{yv} of header fields: /* a{yv} of header fields:
* (things start to be even more invalid below here) */ * (things start to be invalid below here) */
0x20, 0x20, 0x20, 0x00, /* array length (in bytes) */ 0xd0, 0x00, 0x00, 0x00, /* array length (in bytes), must be a multiple of 8 */
0x20, /* array key (this is not currently a valid header field) */ 0x20, /* array key (this is not currently a valid header field) */
/* Variant array value: */ /* Variant array value: */
0x01, /* signature length */ 0x01, /* signature length */
@ -1333,10 +1367,18 @@ test_message_parse_deep_header_nesting (void)
0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
/* Some arbitrary valid content inside the innermost variant: */ /* Some arbitrary valid content inside the innermost variant: */
0x01, 'y', 0x00, 0xcc, 0x01, 'y', 0x00, 0xcc,
/* (message body length missing) */ /* no padding needed as this header element length is a multiple of 8 */
0x05, /* array key (REPLY_SERIAL, required for method return messages) */
/* Variant array value: */
0x01, /* signature length */
'u', /* one complete type */
0x00, /* nul terminator */
/* (Variant array value payload) */
0x00, 0x01, 0x02, 0x03,
/* (message body is zero-length) */
}; };
gsize size = sizeof (data); gsize size = sizeof (data);
GDBusMessage *message = NULL; GDBusMessage *message = NULL;
@ -1362,22 +1404,30 @@ test_message_parse_deep_body_nesting (void)
{ {
const guint8 data[] = { const guint8 data[] = {
'l', /* little-endian byte order */ 'l', /* little-endian byte order */
0x20, /* message type */ 0x02, /* message type (method return) */
0x20, /* message flags */ 0x00, /* message flags (none) */
0x01, /* major protocol version */ 0x01, /* major protocol version */
0x20, 0x20, 0x20, 0x00, /* body length (invalid) */ 0xc4, 0x00, 0x00, 0x00, /* body length (in bytes) */
0x20, 0x20, 0x20, 0x20, /* message serial */ 0x20, 0x20, 0x20, 0x20, /* message serial */
/* a{yv} of header fields: */ /* a{yv} of header fields: */
0x07, 0x00, 0x00, 0x00, /* array length (in bytes) */ 0x10, 0x00, 0x00, 0x00, /* array length (in bytes), must be a multiple of 8 */
0x08, /* array key (signature field) */ 0x08, /* array key (SIGNATURE) */
/* Variant array value: */ /* Variant array value: */
0x01, /* signature length */ 0x01, /* signature length */
'g', /* one complete type */ 'g', /* one complete type */
0x00, /* nul terminator */ 0x00, /* nul terminator */
/* (Variant array value payload) */ /* (Variant array value payload) */
0x01, 'v', 0x00, 0x01, 'v', 0x00,
/* End-of-header padding to reach an 8-byte boundary: */ /* alignment padding before the next header array element, as structs must
0x00, * be 8-aligned: */
0x00,
0x05, /* array key (REPLY_SERIAL, required for method return messages) */
/* Variant array value: */
0x01, /* signature length */
'u', /* one complete type */
0x00, /* nul terminator */
/* (Variant array value payload) */
0x00, 0x01, 0x02, 0x03,
/* Message body: over 64 levels of nested variant, which is not valid: */ /* Message body: over 64 levels of nested variant, which is not valid: */
0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,