[PATCH 1/3] fix code generation for emit_size_read() to check for buffer overflow

[Thomas Haller]

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

Reported-by: Florian Weimer <fweimer at redhat.com>
Signed-off-by: Thomas Haller <thaller at redhat.com>
---
 build-aux/qmi-codegen/VariableArray.py  | 22 ++++++++++++++--------
 build-aux/qmi-codegen/VariableString.py | 22 ++++++++++++++--------
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/build-aux/qmi-codegen/VariableArray.py b/build-aux/qmi-codegen/VariableArray.py
index c402da1..4a9f770 100644
--- a/build-aux/qmi-codegen/VariableArray.py
+++ b/build-aux/qmi-codegen/VariableArray.py
@@ -235,7 +235,8 @@ class VariableArray(Variable):
             translations['fixed_size'] = self.fixed_size
 
             template = (
-                '${lp}    guint16 ${common_var_prefix}_n_items = ${fixed_size};\n')
+                '${lp}    {\n'
+                '${lp}        guint16 ${common_var_prefix}_n_items = ${fixed_size};\n')
             f.write(string.Template(template).substitute(translations))
         else:
             translations['array_size_element_format'] = self.array_size_element.public_format
@@ -250,10 +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'
                 '\n'
-                '${lp}    ${variable_name} += ${array_size_element_size};\n')
+                '${lp}    if ((${variable_name} >= ${buffer_len}) || (${buffer_len} - ${variable_name} < ${array_size_element_size})) {\n'
+                '${lp}        ${variable_name} += ${array_size_element_size};\n'
+                '${lp}    } else {\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'
+                '\n'
+                '${lp}        ${variable_name} += ${array_size_element_size};\n')
 
             if self.array_sequence_element != '':
                 if self.array_sequence_element.public_format == 'guint8':
@@ -266,21 +271,22 @@ class VariableArray(Variable):
                     translations['array_sequence_element_size'] = '0'
                 template += (
                     '\n'
-                    '${lp}    ${variable_name} += ${array_sequence_element_size};\n')
+                    '${lp}        ${variable_name} += ${array_sequence_element_size};\n')
 
             f.write(string.Template(template).substitute(translations))
 
-            self.array_size_element.emit_buffer_read(f, line_prefix + '    ', common_var_prefix + '_n_items', common_var_prefix + '_aux_buffer', common_var_prefix + '_aux_buffer_len')
+            self.array_size_element.emit_buffer_read(f, line_prefix + '        ', common_var_prefix + '_n_items', common_var_prefix + '_aux_buffer', common_var_prefix + '_aux_buffer_len')
 
         template = (
             '\n'
-            '${lp}    for (${common_var_prefix}_i = 0; ${common_var_prefix}_i < ${common_var_prefix}_n_items; ${common_var_prefix}_i++) {\n'
+            '${lp}        for (${common_var_prefix}_i = 0; ${common_var_prefix}_i < ${common_var_prefix}_n_items; ${common_var_prefix}_i++) {\n'
             '\n')
         f.write(string.Template(template).substitute(translations))
 
-        self.array_element.emit_size_read(f, line_prefix + '        ', variable_name, buffer_name, buffer_len)
+        self.array_element.emit_size_read(f, line_prefix + '            ', variable_name, buffer_name, buffer_len)
 
         template = (
+            '${lp}        }\n'
             '${lp}    }\n'
             '${lp}}\n')
         f.write(string.Template(template).substitute(translations))
diff --git a/build-aux/qmi-codegen/VariableString.py b/build-aux/qmi-codegen/VariableString.py
index faa2085..a813a6f 100644
--- a/build-aux/qmi-codegen/VariableString.py
+++ b/build-aux/qmi-codegen/VariableString.py
@@ -120,21 +120,27 @@ class VariableString(Variable):
         elif self.length_prefix_size == 8:
             template = (
                 '${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}    guint8 size8 = 0;\n'
                 '\n'
-                '${lp}    qmi_utils_read_guint8_from_buffer (&aux_buffer, &aux_buffer_len, &size8);\n'
+                '${lp}    if (${variable_name} < ${buffer_len}) {\n'
+                '${lp}        const guint8 *aux_buffer = &${buffer_name}[${variable_name}];\n'
+                '${lp}        guint16 aux_buffer_len = ${buffer_len} - ${variable_name};\n'
+                '\n'
+                '${lp}        qmi_utils_read_guint8_from_buffer (&aux_buffer, &aux_buffer_len, &size8);\n'
+                '${lp}    }\n'
                 '${lp}    ${variable_name} += (1 + size8);\n'
                 '${lp}}\n')
         elif self.length_prefix_size == 16:
             template = (
                 '${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 size16 = 0;\n'
+                '\n'
+                '${lp}    if (${variable_name} < ${buffer_len}) {\n'
+                '${lp}        const guint8 *aux_buffer = &${buffer_name}[${variable_name}];\n'
+                '${lp}        guint16 aux_buffer_len = ${buffer_len} - ${variable_name};\n'
                 '\n'
-                '${lp}    qmi_utils_read_guint16_from_buffer (&aux_buffer, &aux_buffer_len, QMI_ENDIAN_LITTLE, &size16);\n'
+                '${lp}        qmi_utils_read_guint16_from_buffer (&aux_buffer, &aux_buffer_len, QMI_ENDIAN_LITTLE, &size16);\n'
+                '${lp}    }\n'
                 '${lp}    ${variable_name} += (2 + size16);\n'
                 '${lp}}\n')
         f.write(string.Template(template).substitute(translations))
-- 
1.9.3