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>

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 ();