[PATCH 3/3] avoid buffer overlow in emit_input_tlv_add()
Thomas Haller
thaller at redhat.com
Mon Oct 6 06:15:34 PDT 2014
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/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
More information about the libqmi-devel
mailing list