diff options
author | Aleksander Morgado <aleksander@aleksander.es> | 2014-10-07 12:28:46 +0200 |
---|---|---|
committer | Aleksander Morgado <aleksander@aleksander.es> | 2014-10-08 10:37:22 +0200 |
commit | 471d038fe38f7b99383f9654dcc8f6662d96e6f8 (patch) | |
tree | 0b605ec0c17afb1c469597689d52950d0aef53a8 | |
parent | 0cf3aa3adf0a9940b1d37eb46f54d6af013ac5fe (diff) | |
download | external_libqmi-471d038fe38f7b99383f9654dcc8f6662d96e6f8.zip external_libqmi-471d038fe38f7b99383f9654dcc8f6662d96e6f8.tar.gz external_libqmi-471d038fe38f7b99383f9654dcc8f6662d96e6f8.tar.bz2 |
qmi-codegen: ensure enough buffer available to read string/array size variable
Code generation via emit_size_read() creates the _validate() functions.
The generated code for strings and arrays used to read the length prefix
without checking that the provided buffer is large enough.
https://bugzilla.redhat.com/show_bug.cgi?id=1031738
Patch based on a patch from Thomas Haller <thaller@redhat.com>
Reported-by: Florian Weimer <fweimer@redhat.com>
-rw-r--r-- | build-aux/qmi-codegen/Field.py | 5 | ||||
-rw-r--r-- | build-aux/qmi-codegen/VariableArray.py | 9 | ||||
-rw-r--r-- | build-aux/qmi-codegen/VariableString.py | 18 | ||||
-rw-r--r-- | src/libqmi-glib/test/test-message.c | 28 |
4 files changed, 56 insertions, 4 deletions
diff --git a/build-aux/qmi-codegen/Field.py b/build-aux/qmi-codegen/Field.py index a3f3a61..ddbcfe1 100644 --- a/build-aux/qmi-codegen/Field.py +++ b/build-aux/qmi-codegen/Field.py @@ -339,7 +339,10 @@ class Field: '\n') f.write(string.Template(template).substitute(translations)) - # Now, read the size of the expected TLV + # Now, read the size of the expected TLV. + # + # Note: the emit_size_read() implementation is allowed to return FALSE + # to indicate an error at any time. self.variable.emit_size_read(f, ' ', 'expected_len', 'buffer', 'buffer_len') template = ( diff --git a/build-aux/qmi-codegen/VariableArray.py b/build-aux/qmi-codegen/VariableArray.py index c402da1..7f38202 100644 --- a/build-aux/qmi-codegen/VariableArray.py +++ b/build-aux/qmi-codegen/VariableArray.py @@ -251,7 +251,14 @@ class VariableArray(Variable): template = ( '${lp} ${array_size_element_format} ${common_var_prefix}_n_items;\n' '${lp} const guint8 *${common_var_prefix}_aux_buffer = &${buffer_name}[${variable_name}];\n' - '${lp} guint16 ${common_var_prefix}_aux_buffer_len = ${buffer_len} - ${variable_name};\n' + '${lp} guint16 ${common_var_prefix}_aux_buffer_len;\n' + '\n' + '${lp} ${common_var_prefix}_aux_buffer_len = ((${buffer_len} >= ${variable_name}) ? ${buffer_len} - ${variable_name} : 0);\n' + '${lp} if (${common_var_prefix}_aux_buffer_len < ${array_size_element_size}) {\n' + '${lp} g_warning ("Cannot read the array size: expected \'%u\' bytes, but only got \'%u\' bytes",\n' + '${lp} ${array_size_element_size}, ${common_var_prefix}_aux_buffer_len);\n' + '${lp} return FALSE;\n' + '${lp} }\n' '\n' '${lp} ${variable_name} += ${array_size_element_size};\n') diff --git a/build-aux/qmi-codegen/VariableString.py b/build-aux/qmi-codegen/VariableString.py index faa2085..0ea3bd3 100644 --- a/build-aux/qmi-codegen/VariableString.py +++ b/build-aux/qmi-codegen/VariableString.py @@ -122,7 +122,14 @@ class VariableString(Variable): '${lp}{\n' '${lp} guint8 size8;\n' '${lp} const guint8 *aux_buffer = &${buffer_name}[${variable_name}];\n' - '${lp} guint16 aux_buffer_len = ${buffer_len} - ${variable_name};\n' + '${lp} guint16 aux_buffer_len;\n' + '\n' + '${lp} aux_buffer_len = ((${buffer_len} >= ${variable_name}) ? ${buffer_len} - ${variable_name} : 0);\n' + '${lp} if (aux_buffer_len < 1) {\n' + '${lp} g_warning ("Cannot read the string size: expected \'1\' bytes, but only got \'%u\' bytes",\n' + '${lp} aux_buffer_len);\n' + '${lp} return FALSE;\n' + '${lp} }\n' '\n' '${lp} qmi_utils_read_guint8_from_buffer (&aux_buffer, &aux_buffer_len, &size8);\n' '${lp} ${variable_name} += (1 + size8);\n' @@ -132,7 +139,14 @@ class VariableString(Variable): '${lp}{\n' '${lp} guint16 size16;\n' '${lp} const guint8 *aux_buffer = &${buffer_name}[${variable_name}];\n' - '${lp} guint16 aux_buffer_len = ${buffer_len} - ${variable_name};\n' + '${lp} guint16 aux_buffer_len;\n' + '\n' + '${lp} aux_buffer_len = ((${buffer_len} >= ${variable_name}) ? ${buffer_len} - ${variable_name} : 0);\n' + '${lp} if (aux_buffer_len < 2) {\n' + '${lp} g_warning ("Cannot read the string size: expected \'2\' bytes, but only got \'%u\' bytes",\n' + '${lp} aux_buffer_len);\n' + '${lp} return FALSE;\n' + '${lp} }\n' '\n' '${lp} qmi_utils_read_guint16_from_buffer (&aux_buffer, &aux_buffer_len, QMI_ENDIAN_LITTLE, &size16);\n' '${lp} ${variable_name} += (2 + size16);\n' diff --git a/src/libqmi-glib/test/test-message.c b/src/libqmi-glib/test/test-message.c index 86fed8a..f78b647 100644 --- a/src/libqmi-glib/test/test-message.c +++ b/src/libqmi-glib/test/test-message.c @@ -131,6 +131,33 @@ test_message_parse_wrong_tlv (void) test_message_parse_common (buffer, sizeof (buffer), 1); g_test_assert_expected_messages (); } + +static void +test_message_parse_missing_size (void) +{ + /* PDS Event Report indication: NMEA position */ + const guint8 buffer[] = { + 0x01, /* marker */ + 0x10, 0x00, /* qmux length */ + 0x80, /* qmux flags */ + 0x06, /* service: PDS */ + 0x03, /* client */ + 0x04, /* service flags: Indication */ + 0x01, 0x00, /* transaction */ + 0x01, 0x00, /* message: Event Report */ + 0x04, 0x00, /* all tlvs length: 4 bytes */ + /* TLV */ + 0x11, /* type: Extended NMEA Position (1 guint8 and one 16-bit-sized string) */ + 0x01, 0x00, /* length: 1 byte (WE ONLY GIVE THE GUINT8!!!) */ + 0x01 + }; + + g_test_expect_message ("Qmi", + G_LOG_LEVEL_WARNING, + "Cannot read the string size: expected '*' bytes, but only got '*' bytes"); + test_message_parse_common (buffer, sizeof (buffer), 1); + g_test_assert_expected_messages (); +} #endif int main (int argc, char **argv) @@ -144,6 +171,7 @@ int main (int argc, char **argv) g_test_add_func ("/libqmi-glib/message/parse/complete-and-complete", test_message_parse_complete_and_complete); #if GLIB_CHECK_VERSION (2,34,0) g_test_add_func ("/libqmi-glib/message/parse/wrong-tlv", test_message_parse_wrong_tlv); + g_test_add_func ("/libqmi-glib/message/parse/missing-size", test_message_parse_missing_size); #endif return g_test_run (); |