[PATCH 3/3] avoid buffer overlow in emit_input_tlv_add()
Aleksander Morgado
aleksander at aleksander.es
Wed Oct 8 02:20:13 PDT 2014
On Mon, Oct 6, 2014 at 3:15 PM, Thomas Haller <thaller at redhat.com> wrote:
> 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>
In git master now as well.
> ---
> build-aux/qmi-codegen/Field.py | 12 +++++++-
> build-aux/qmi-codegen/Variable.py | 2 +-
> build-aux/qmi-codegen/VariableArray.py | 8 +++---
> build-aux/qmi-codegen/VariableInteger.py | 46 ++++++++++++++++++++++++++-----
> build-aux/qmi-codegen/VariableSequence.py | 4 +--
> build-aux/qmi-codegen/VariableString.py | 29 +++++++++++--------
> build-aux/qmi-codegen/VariableStruct.py | 4 +--
> 7 files changed, 77 insertions(+), 28 deletions(-)
>
> diff --git a/build-aux/qmi-codegen/Field.py b/build-aux/qmi-codegen/Field.py
> index a3f3a61..4a428fc 100644
> --- a/build-aux/qmi-codegen/Field.py
> +++ b/build-aux/qmi-codegen/Field.py
> @@ -216,9 +216,11 @@ class Field:
> Emit the code responsible for adding the TLV to the QMI message
> """
> def emit_input_tlv_add(self, f, line_prefix):
> + error_label = 'ERR_EMIT_BUFFER_OVERFLOW_' + self.id_enum_name
> translations = { 'name' : self.name,
> 'tlv_id' : self.id_enum_name,
> 'variable_name' : self.variable_name,
> + 'error_label' : error_label,
> 'lp' : line_prefix }
>
> template = (
> @@ -229,15 +231,23 @@ class Field:
> f.write(string.Template(template).substitute(translations))
>
> # Now, write the contents of the variable into the buffer
> - self.variable.emit_buffer_write(f, line_prefix, 'input->' + self.variable_name, 'buffer_aux', 'buffer_len')
> + self.variable.emit_buffer_write(f, line_prefix, 'input->' + self.variable_name, 'buffer_aux', 'buffer_len', error_label)
>
> template = (
> '\n'
> + '${lp}if (FALSE) {\n'
> + '${lp} ${error_label}:\n'
> + '${lp} g_set_error (error, QMI_CORE_ERROR, QMI_CORE_ERROR_TLV_TOO_LONG, "result larger then 1024 bytes");\n'
> + '${lp} goto OUT_${error_label};\n'
> + '${lp}}\n'
> + '\n'
> + '${lp}g_assert (buffer_len <= 1024);\n'
> '${lp}if (!qmi_message_add_raw_tlv (self,\n'
> '${lp} (guint8)${tlv_id},\n'
> '${lp} buffer,\n'
> '${lp} (1024 - buffer_len),\n'
> '${lp} error)) {\n'
> + '${lp} OUT_${error_label}:\n'
> '${lp} g_prefix_error (error, \"Couldn\'t set the ${name} TLV: \");\n'
> '${lp} qmi_message_unref (self);\n'
> '${lp} return NULL;\n'
> diff --git a/build-aux/qmi-codegen/Variable.py b/build-aux/qmi-codegen/Variable.py
> index c238471..fbc93f5 100644
> --- a/build-aux/qmi-codegen/Variable.py
> +++ b/build-aux/qmi-codegen/Variable.py
> @@ -81,7 +81,7 @@ class Variable:
> Emits the code involved in writing the variable to the raw byte stream
> from the specific private format.
> """
> - def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len):
> + def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len, error_label):
> pass
>
>
> diff --git a/build-aux/qmi-codegen/VariableArray.py b/build-aux/qmi-codegen/VariableArray.py
> index 4a9f770..1259cdb 100644
> --- a/build-aux/qmi-codegen/VariableArray.py
> +++ b/build-aux/qmi-codegen/VariableArray.py
> @@ -295,7 +295,7 @@ class VariableArray(Variable):
> Writing an array to the raw byte buffer is just about providing a loop to
> write every array element one by one.
> """
> - def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len):
> + def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len, error_label):
> common_var_prefix = utils.build_underscore_name(self.name)
> translations = { 'lp' : line_prefix,
> 'variable_name' : variable_name,
> @@ -318,10 +318,10 @@ class VariableArray(Variable):
> '${lp} ${common_var_prefix}_n_items = (${array_size_element_format}) ${variable_name}->len;\n')
> f.write(string.Template(template).substitute(translations))
>
> - self.array_size_element.emit_buffer_write(f, line_prefix + ' ', common_var_prefix + '_n_items', buffer_name, buffer_len)
> + self.array_size_element.emit_buffer_write(f, line_prefix + ' ', common_var_prefix + '_n_items', buffer_name, buffer_len, error_label)
>
> if self.array_sequence_element != '':
> - self.array_sequence_element.emit_buffer_write(f, line_prefix + ' ', variable_name + '_sequence', buffer_name, buffer_len)
> + self.array_sequence_element.emit_buffer_write(f, line_prefix + ' ', variable_name + '_sequence', buffer_name, buffer_len, error_label)
>
>
> template = (
> @@ -329,7 +329,7 @@ class VariableArray(Variable):
> '${lp} for (${common_var_prefix}_i = 0; ${common_var_prefix}_i < ${variable_name}->len; ${common_var_prefix}_i++) {\n')
> f.write(string.Template(template).substitute(translations))
>
> - self.array_element.emit_buffer_write(f, line_prefix + ' ', 'g_array_index (' + variable_name + ', ' + self.array_element.public_format + ',' + common_var_prefix + '_i)', buffer_name, buffer_len)
> + self.array_element.emit_buffer_write(f, line_prefix + ' ', 'g_array_index (' + variable_name + ', ' + self.array_element.public_format + ',' + common_var_prefix + '_i)', buffer_name, buffer_len, error_label)
>
> template = (
> '${lp} }\n'
> diff --git a/build-aux/qmi-codegen/VariableInteger.py b/build-aux/qmi-codegen/VariableInteger.py
> index 235b552..37e587d 100644
> --- a/build-aux/qmi-codegen/VariableInteger.py
> +++ b/build-aux/qmi-codegen/VariableInteger.py
> @@ -130,22 +130,47 @@ class VariableInteger(Variable):
> '${lp}${variable_name} += 8;\n')
> f.write(string.Template(template).substitute(translations))
>
> + """
> + Return the data type size of fixed c-types
> + """
> + @staticmethod
> + def fixed_type_byte_size(fmt):
> + if fmt == 'guint8':
> + return 1
> + if fmt == 'guint16':
> + return 2
> + if fmt == 'guint32':
> + return 4
> + if fmt == 'guint64':
> + return 8
> + if fmt == 'gint8':
> + return 1
> + if fmt == 'gint16':
> + return 2
> + if fmt == 'gint32':
> + return 4
> + if fmt == 'gint64':
> + return 8
> + raise Exception("Unsupported format %s" % (fmt))
>
> """
> Write a single integer to the raw byte buffer
> """
> - def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len):
> + def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len, error_label):
> translations = { 'lp' : line_prefix,
> 'private_format' : self.private_format,
> 'len' : self.guint_sized_size,
> 'variable_name' : variable_name,
> 'buffer_name' : buffer_name,
> 'buffer_len' : buffer_len,
> + 'error_label' : error_label,
> 'endian' : self.endian }
>
> if self.format == 'guint-sized':
> template = (
> '${lp}/* Write the ${len}-byte long variable to the buffer */\n'
> + '${lp}if (${buffer_len} < ${len})\n'
> + '${lp} goto ${error_label};\n'
> '${lp}qmi_utils_write_sized_guint_to_buffer (\n'
> '${lp} &${buffer_name},\n'
> '${lp} &${buffer_len},\n'
> @@ -153,19 +178,26 @@ class VariableInteger(Variable):
> '${lp} ${endian},\n'
> '${lp} &(${variable_name}));\n')
> elif self.private_format == self.public_format:
> + translations['byte_size'] = VariableInteger.fixed_type_byte_size(self.private_format)
> template = (
> '${lp}/* Write the ${private_format} variable to the buffer */\n'
> - '${lp}qmi_utils_write_${private_format}_to_buffer (\n'
> - '${lp} &${buffer_name},\n'
> - '${lp} &${buffer_len},\n')
> + '${lp}if (${buffer_len} < ${byte_size})\n'
> + '${lp} goto ${error_label};\n'
> + '${lp}else\n'
> + '${lp} qmi_utils_write_${private_format}_to_buffer (\n'
> + '${lp} &${buffer_name},\n'
> + '${lp} &${buffer_len},\n')
> if self.private_format != 'guint8' and self.private_format != 'gint8':
> template += (
> - '${lp} ${endian},\n')
> + '${lp} ${endian},\n')
> template += (
> - '${lp} &(${variable_name}));\n')
> + '${lp} &(${variable_name}));\n')
> else:
> + translations['byte_size'] = VariableInteger.fixed_type_byte_size(self.private_format)
> template = (
> - '${lp}{\n'
> + '${lp}if (${buffer_len} < ${byte_size})\n'
> + '${lp} goto ${error_label};\n'
> + '${lp}else {\n'
> '${lp} ${private_format} tmp;\n'
> '\n'
> '${lp} tmp = (${private_format})${variable_name};\n'
> diff --git a/build-aux/qmi-codegen/VariableSequence.py b/build-aux/qmi-codegen/VariableSequence.py
> index 0ea5a9d..bacc541 100644
> --- a/build-aux/qmi-codegen/VariableSequence.py
> +++ b/build-aux/qmi-codegen/VariableSequence.py
> @@ -92,9 +92,9 @@ class VariableSequence(Variable):
> Writing the contents of a sequence is just about writing each of the sequence
> fields one by one.
> """
> - def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len):
> + def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len, error_label):
> for member in self.members:
> - member['object'].emit_buffer_write(f, line_prefix, variable_name + '_' + member['name'], buffer_name, buffer_len)
> + member['object'].emit_buffer_write(f, line_prefix, variable_name + '_' + member['name'], buffer_name, buffer_len, error_label)
>
>
> """
> diff --git a/build-aux/qmi-codegen/VariableString.py b/build-aux/qmi-codegen/VariableString.py
> index a813a6f..0254210 100644
> --- a/build-aux/qmi-codegen/VariableString.py
> +++ b/build-aux/qmi-codegen/VariableString.py
> @@ -149,30 +149,37 @@ class VariableString(Variable):
> """
> Write a string to the raw byte buffer.
> """
> - def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len):
> + def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len, error_label):
> translations = { 'lp' : line_prefix,
> 'variable_name' : variable_name,
> 'buffer_name' : buffer_name,
> + 'error_label' : error_label,
> 'buffer_len' : buffer_len }
>
> if self.is_fixed_size:
> translations['fixed_size'] = self.fixed_size
> template = (
> '${lp}/* Write the fixed-size string variable to the buffer */\n'
> - '${lp}qmi_utils_write_fixed_size_string_to_buffer (\n'
> - '${lp} &${buffer_name},\n'
> - '${lp} &${buffer_len},\n'
> - '${lp} ${fixed_size},\n'
> - '${lp} ${variable_name});\n')
> + '${lp}if (${buffer_len} < ${fixed_size})\n'
> + '${lp} goto ${error_label};\n'
> + '${lp}else\n'
> + '${lp} qmi_utils_write_fixed_size_string_to_buffer (\n'
> + '${lp} &${buffer_name},\n'
> + '${lp} &${buffer_len},\n'
> + '${lp} ${fixed_size},\n'
> + '${lp} ${variable_name});\n')
> else:
> translations['length_prefix_size'] = self.length_prefix_size
> template = (
> '${lp}/* Write the string variable to the buffer */\n'
> - '${lp}qmi_utils_write_string_to_buffer (\n'
> - '${lp} &${buffer_name},\n'
> - '${lp} &${buffer_len},\n'
> - '${lp} ${length_prefix_size},\n'
> - '${lp} ${variable_name});\n')
> + '${lp}if (!${variable_name} || ${buffer_len} < ${length_prefix_size} + strlen (${variable_name}))\n'
> + '${lp} goto ${error_label};\n'
> + '${lp}else\n'
> + '${lp} qmi_utils_write_string_to_buffer (\n'
> + '${lp} &${buffer_name},\n'
> + '${lp} &${buffer_len},\n'
> + '${lp} ${length_prefix_size},\n'
> + '${lp} ${variable_name});\n')
>
> f.write(string.Template(template).substitute(translations))
>
> diff --git a/build-aux/qmi-codegen/VariableStruct.py b/build-aux/qmi-codegen/VariableStruct.py
> index 522551e..041e6ab 100644
> --- a/build-aux/qmi-codegen/VariableStruct.py
> +++ b/build-aux/qmi-codegen/VariableStruct.py
> @@ -121,9 +121,9 @@ class VariableStruct(Variable):
> Writing the contents of a struct is just about writing each of the struct
> fields one by one.
> """
> - def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len):
> + def emit_buffer_write(self, f, line_prefix, variable_name, buffer_name, buffer_len, error_label):
> for member in self.members:
> - member['object'].emit_buffer_write(f, line_prefix, variable_name + '.' + member['name'], buffer_name, buffer_len)
> + member['object'].emit_buffer_write(f, line_prefix, variable_name + '.' + member['name'], buffer_name, buffer_len, error_label)
>
>
> """
> --
> 1.9.3
>
--
Aleksander
https://aleksander.es
More information about the libqmi-devel
mailing list