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

D-Bus: Improve signature validation 

An invalid signature ("a{sa}") caused a segfault when the array inside
the entry had a length of zero. An array signature code ("a") must be
followed by a single complete type, and "}" is not one of them. Check
additional restrictions for structs and dict entries, which aren't
related to this bug.

Fixes #17176
This commit is contained in:
Simon Holesch 2021-01-28 01:17:54 +01:00
parent 266e99e11a
commit 6508b02ec4
1 changed files with 75 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
epan/dissectors/packet-dbus.c
View File

@ -382,10 +382,60 @@ is_basic_type(char sig_code) {
}
}
static const char *
skip_enclosed_container(const char *signature, char open_bracket, char closed_bracket) {
int nested = 0;
for (char sig_code = *signature++; sig_code != '\0'; sig_code = *signature++) {
if (sig_code == closed_bracket) {
if (nested == 0) {
return signature;
}
nested--;
} else if (sig_code == open_bracket) {
nested++;
}
}
return NULL;
}
static const char *
skip_single_complete_type(const char *signature) {
char sig_code;
while (1) {
sig_code = *signature++;
switch (sig_code) {
case SIG_CODE_BYTE:
case SIG_CODE_BOOLEAN:
case SIG_CODE_INT16:
case SIG_CODE_UINT16:
case SIG_CODE_INT32:
case SIG_CODE_UINT32:
case SIG_CODE_INT64:
case SIG_CODE_UINT64:
case SIG_CODE_DOUBLE:
case SIG_CODE_STRING:
case SIG_CODE_OBJECT_PATH:
case SIG_CODE_SIGNATURE:
case SIG_CODE_VARIANT:
case SIG_CODE_UNIX_FD:
return signature;
case SIG_CODE_ARRAY:
continue;
case SIG_CODE_STRUCT_OPEN:
return skip_enclosed_container(signature, SIG_CODE_STRUCT_OPEN, SIG_CODE_STRUCT_CLOSE);
case SIG_CODE_DICT_ENTRY_OPEN:
return skip_enclosed_container(signature, SIG_CODE_DICT_ENTRY_OPEN, SIG_CODE_DICT_ENTRY_CLOSE);
default:
return NULL;
}
}
}
static gboolean
is_dbus_signature_valid(const char *signature) {
char sig_code;
size_t length = 0;
char prev_sig_code = '\0';
wmem_stack_t *expected_chars = wmem_stack_new(wmem_packet_scope());
while ((sig_code = *signature++) != '\0') {
@ -410,21 +460,38 @@ is_dbus_signature_valid(const char *signature) {
case SIG_CODE_DOUBLE:
break;
case SIG_CODE_ARRAY:
if (*signature == '\0') {
switch (*signature) {
case '\0':
case SIG_CODE_STRUCT_CLOSE:
case SIG_CODE_DICT_ENTRY_CLOSE:
// arrays must be followed by a single complete type
return FALSE;
}
// invalid signature codes are detected in the next iteration
break;
case SIG_CODE_STRUCT_OPEN:
if (*signature == SIG_CODE_STRUCT_CLOSE) {
// empty structures are not allowed
return FALSE;
}
wmem_stack_push(expected_chars, (void *)SIG_CODE_STRUCT_CLOSE);
break;
case SIG_CODE_DICT_ENTRY_OPEN:
if (!is_basic_type(*signature)) {
// key of dict entry must be a basic type
case SIG_CODE_DICT_ENTRY_OPEN: {
// dict entries must be an array element type
// the first single complete type (the "key") must be a basic type
if (prev_sig_code != SIG_CODE_ARRAY || !is_basic_type(*signature)) {
return FALSE;
}
// dict entries must contain exactly two single complete types
// + 1 can be used here, since the key is a basic type
const char *sig_code_close = skip_single_complete_type(signature + 1);
if (!sig_code_close || *sig_code_close != SIG_CODE_DICT_ENTRY_CLOSE) {
return FALSE;
}
wmem_stack_push(expected_chars, (void *)SIG_CODE_DICT_ENTRY_CLOSE);
break;
}
case SIG_CODE_STRUCT_CLOSE:
case SIG_CODE_DICT_ENTRY_CLOSE:
if (wmem_stack_count(expected_chars) == 0 ||
@ -435,6 +502,8 @@ is_dbus_signature_valid(const char *signature) {
default:
return FALSE;
}
prev_sig_code = sig_code;
}
return wmem_stack_count(expected_chars) == 0;
}
@ -536,55 +605,6 @@ add_padding(dbus_packet_t *packet, char sig) {
return 0;
}
static const char *
skip_enclosed_container(const char *signature, char open_bracket, char closed_bracket) {
int nested = 0;
for (char sig_code = *signature++; sig_code != '\0'; sig_code = *signature++) {
if (sig_code == closed_bracket) {
if (nested == 0) {
return signature;
}
nested--;
} else if (sig_code == open_bracket) {
nested++;
}
}
return NULL;
}
static const char *
skip_single_complete_type(const char *signature) {
char sig_code;
while (1) {
sig_code = *signature++;
switch (sig_code) {
case SIG_CODE_BYTE:
case SIG_CODE_BOOLEAN:
case SIG_CODE_INT16:
case SIG_CODE_UINT16:
case SIG_CODE_INT32:
case SIG_CODE_UINT32:
case SIG_CODE_INT64:
case SIG_CODE_UINT64:
case SIG_CODE_DOUBLE:
case SIG_CODE_STRING:
case SIG_CODE_OBJECT_PATH:
case SIG_CODE_SIGNATURE:
case SIG_CODE_VARIANT:
case SIG_CODE_UNIX_FD:
return signature;
case SIG_CODE_ARRAY:
continue;
case SIG_CODE_STRUCT_OPEN:
return skip_enclosed_container(signature, SIG_CODE_STRUCT_OPEN, SIG_CODE_STRUCT_CLOSE);
case SIG_CODE_DICT_ENTRY_OPEN:
return skip_enclosed_container(signature, SIG_CODE_DICT_ENTRY_OPEN, SIG_CODE_DICT_ENTRY_CLOSE);
default:
return NULL;
}
}
}
static void
reader_cleanup(dbus_type_reader_t *reader) {
for (dbus_type_reader_t *r = reader; r->parent; r = r->parent) {
@ -697,9 +717,8 @@ reader_next(dbus_type_reader_t *reader, int hf, int ett, dbus_val_t *value) {
add_padding(packet, *reader->signature);
if (array_len == 0) {
reader->signature = skip_single_complete_type(reader->signature);
if (reader->signature == NULL) {
err = 1;
}
// all signatures are validated
DISSECTOR_ASSERT(reader->signature);
ptvcursor_pop_subtree(packet->cursor);
is_single_complete_type = TRUE;
} else if (array_len <= DBUS_MAX_ARRAY_LEN) {