libmbim-glib,message: reading guint32 may fail

6 files changed, 82 insertions, 36 deletions

diff --git a/build-aux/mbim-codegen/Message.py b/build-aux/mbim-codegen/Message.py
index 49cbf0e..fd6e4bb 100644
--- a/build-aux/mbim-codegen/Message.py
+++ b/build-aux/mbim-codegen/Message.py
@@ -738,8 +738,10 @@ class Message:
                     '    {\n')
 
             if 'always-read' in field:
+                count_early_outs += 1
                 inner_template += (
-                    '        _${field} = _mbim_message_read_guint32 (message, offset);\n'
+                    '        if (!_mbim_message_read_guint32 (message, offset, &_${field}, error))\n'
+                    '            goto out;\n'
                     '        if (out_${field} != NULL)\n'
                     '            *out_${field} = _${field};\n'
                     '        offset += 4;\n')
@@ -786,9 +788,22 @@ class Message:
                     '            goto out;\n'
                     '        offset += 16;\n')
             elif field['format'] == 'guint32':
+                count_early_outs += 1
+                if 'public-format' in field:
+                    translations['public'] = field['public-format'] if 'public-format' in field else field['format']
+                    inner_template += (
+                        '        if (out_${field} != NULL) {\n'
+                        '            guint32 aux;\n'
+                        '\n'
+                        '            if (!_mbim_message_read_guint32 (message, offset, &aux, error))\n'
+                        '                goto out;\n'
+                        '            *out_${field} = (${public})aux;\n'
+                        '        }\n')
+                else:
+                    inner_template += (
+                        '        if ((out_${field} != NULL) && !_mbim_message_read_guint32 (message, offset, out_${field}, error))\n'
+                        '            goto out;\n')
                 inner_template += (
-                    '        if (out_${field} != NULL)\n'
-                    '            *out_${field} =  _mbim_message_read_guint32 (message, offset);\n'
                     '        offset += 4;\n')
             elif field['format'] == 'guint64':
                 inner_template += (
@@ -954,7 +969,9 @@ class Message:
 
         needs_early_out = False
         for field in fields:
-            if field['format'] == 'uuid' or \
+            if 'always-read' in field or \
+               field['format'] == 'guint32' or \
+               field['format'] == 'uuid' or \
                field['format'] == 'struct-array' or \
                field['format'] == 'ref-struct-array' or \
                field['format'] == 'struct' or \
@@ -1017,7 +1034,8 @@ class Message:
 
             if 'always-read' in field:
                 inner_template += (
-                    '        _${field} = _mbim_message_read_guint32 (message, offset);\n'
+                    '        if (!_mbim_message_read_guint32 (message, offset, &_${field}, &inner_error))\n'
+                    '            goto out;\n'
                     '        offset += 4;\n'
                     '        g_string_append_printf (str, "\'%" G_GUINT32_FORMAT "\'", _${field});\n')
 
@@ -1075,11 +1093,12 @@ class Message:
             elif field['format'] == 'guint32' or \
                  field['format'] == 'guint64':
                 inner_template += (
-                    '        ${public} tmp;\n'
+                    '        ${field_format} tmp;\n'
                     '\n')
                 if field['format'] == 'guint32' :
                     inner_template += (
-                        '        tmp = (${public}) _mbim_message_read_guint32 (message, offset);\n'
+                        '        if (!_mbim_message_read_guint32 (message, offset, &tmp, &inner_error))\n'
+                        '            goto out;\n'
                         '        offset += 4;\n')
                 elif field['format'] == 'guint64' :
                     inner_template += (
@@ -1089,12 +1108,12 @@ class Message:
                 if 'public-format' in field:
                     inner_template += (
                         '#if defined __${public_underscore_upper}_IS_ENUM__\n'
-                        '        g_string_append_printf (str, "\'%s\'", ${public_underscore}_get_string (tmp));\n'
+                        '        g_string_append_printf (str, "\'%s\'", ${public_underscore}_get_string ((${public})tmp));\n'
                         '#elif defined __${public_underscore_upper}_IS_FLAGS__\n'
                         '        {\n'
                         '            gchar *tmpstr;\n'
                         '\n'
-                        '            tmpstr = ${public_underscore}_build_string_from_mask (tmp);\n'
+                        '            tmpstr = ${public_underscore}_build_string_from_mask ((${public})tmp);\n'
                         '            g_string_append_printf (str, "\'%s\'", tmpstr);\n'
                         '            g_free (tmpstr);\n'
                         '        }\n'
diff --git a/build-aux/mbim-codegen/Struct.py b/build-aux/mbim-codegen/Struct.py
index 751f819..e2f16b9 100644
--- a/build-aux/mbim-codegen/Struct.py
+++ b/build-aux/mbim-codegen/Struct.py
@@ -494,9 +494,11 @@ class Struct:
                     '        offset += ${array_size};\n'
                     '    }\n')
             elif field['format'] == 'guint32':
+                count_early_outs += 1
                 inner_template += (
                     '\n'
-                    '    out->${field_name_underscore} = _mbim_message_read_guint32 (self, offset);\n'
+                    '    if (!_mbim_message_read_guint32 (self, offset, &out->${field_name_underscore}, error))\n'
+                    '        goto out;\n'
                     '    offset += 4;\n')
             elif field['format'] == 'guint32-array':
                 translations['array_size_field_name_underscore'] = utils.build_underscore_name_from_camelcase(field['array-size-field'])
@@ -602,13 +604,17 @@ class Struct:
                 '    out = g_new0 (${name} *, array_size + 1);\n'
                 '\n'
                 '    if (!refs) {\n'
-                '        offset = _mbim_message_read_guint32 (self, relative_offset_array_start);\n'
+                '        _mbim_message_read_guint32 (self, relative_offset_array_start, &offset, &inner_error);\n'
                 '        for (i = 0; !inner_error && (i < array_size); i++, offset += ${struct_size})\n'
                 '            out[i] = _mbim_message_read_${name_underscore}_struct (self, offset, NULL, &inner_error);\n'
                 '    } else {\n'
                 '        offset = relative_offset_array_start;\n'
-                '        for (i = 0; !inner_error && (i < array_size); i++, offset += 8)\n'
-                '            out[i] = _mbim_message_read_${name_underscore}_struct (self, _mbim_message_read_guint32 (self, offset), NULL, &inner_error);\n'
+                '        for (i = 0; !inner_error && (i < array_size); i++, offset += 8) {\n'
+                '            guint32 tmp_offset;\n'
+                '\n'
+                '            if (_mbim_message_read_guint32 (self, offset, &tmp_offset, &inner_error))\n'
+                '                out[i] = _mbim_message_read_${name_underscore}_struct (self, tmp_offset, NULL, &inner_error);\n'
+                '        }\n'
                 '    }\n'
                 '\n'
                 '    if (!inner_error) {\n'
diff --git a/src/libmbim-glib/mbim-message-private.h b/src/libmbim-glib/mbim-message-private.h
index 3555b68..55faa78 100644
--- a/src/libmbim-glib/mbim-message-private.h
+++ b/src/libmbim-glib/mbim-message-private.h
@@ -269,9 +269,11 @@ gboolean _mbim_message_read_uuid          (const MbimMessage  *self,
                                            guint32             relative_offset,
                                            const MbimUuid    **uuid,
                                            GError            **error);
+gboolean _mbim_message_read_guint32       (const MbimMessage  *self,
+                                           guint32             relative_offset,
+                                           guint32            *value,
+                                           GError            **error);
 
-guint32          _mbim_message_read_guint32       (const MbimMessage *self,
-                                                   guint32            relative_offset);
 guint32         *_mbim_message_read_guint32_array (const MbimMessage *self,
                                                    guint32            array_size,
                                                    guint32            relative_offset_array_start);
diff --git a/src/libmbim-glib/mbim-message.c b/src/libmbim-glib/mbim-message.c
index 7306213..3872f17 100644
--- a/src/libmbim-glib/mbim-message.c
+++ b/src/libmbim-glib/mbim-message.c
@@ -171,17 +171,32 @@ _mbim_message_get_information_buffer_offset (const MbimMessage *self)
     }
 }
 
-guint32
-_mbim_message_read_guint32 (const MbimMessage *self,
-                            guint32            relative_offset)
+gboolean
+_mbim_message_read_guint32 (const MbimMessage  *self,
+                            guint32             relative_offset,
+                            guint32            *value,
+                            GError            **error)
 {
+    guint32 required_size;
     guint32 information_buffer_offset;
 
+    g_assert (value);
+
     information_buffer_offset = _mbim_message_get_information_buffer_offset (self);
-    return GUINT32_FROM_LE (G_STRUCT_MEMBER (
-                                guint32,
-                                self->data,
-                                (information_buffer_offset + relative_offset)));
+
+    required_size = information_buffer_offset + relative_offset + 4;
+    if (self->len < required_size) {
+        g_set_error (error, MBIM_CORE_ERROR, MBIM_CORE_ERROR_INVALID_MESSAGE,
+                     "cannot read 32bit unsigned integer (4 bytes) (%u < %u)",
+                     self->len, required_size);
+        return FALSE;
+    }
+
+    *value = GUINT32_FROM_LE (G_STRUCT_MEMBER (
+                                  guint32,
+                                  self->data,
+                                  (information_buffer_offset + relative_offset)));
+    return TRUE;
 }
 
 guint32 *
diff --git a/src/libmbim-glib/mbim-proxy-helpers.c b/src/libmbim-glib/mbim-proxy-helpers.c
index a75a8e6..e527801 100644
--- a/src/libmbim-glib/mbim-proxy-helpers.c
+++ b/src/libmbim-glib/mbim-proxy-helpers.c
@@ -139,13 +139,14 @@ _mbim_proxy_helper_service_subscribe_request_parse (MbimMessage  *message,
     guint32 element_count;
     guint32 offset = 0;
     guint32 array_offset;
-    MbimEventEntry *event;
     GError *inner_error = NULL;
 
     g_assert (message != NULL);
     g_assert (out_size != NULL);
 
-    element_count = _mbim_message_read_guint32 (message, offset);
+    if (!_mbim_message_read_guint32 (message, offset, &element_count, error))
+        return NULL;
+
     if (element_count) {
         array = g_new0 (MbimEventEntry *, element_count + 1);
 
@@ -153,24 +154,21 @@ _mbim_proxy_helper_service_subscribe_request_parse (MbimMessage  *message,
         for (i = 0; i < element_count; i++) {
             const MbimUuid *uuid;
 
-            array_offset = _mbim_message_read_guint32 (message, offset);
-
+            if (!_mbim_message_read_guint32 (message, offset, &array_offset, &inner_error))
+                break;
             if (!_mbim_message_read_uuid (message, array_offset, &uuid, &inner_error))
                 break;
 
-            event = g_new (MbimEventEntry, 1);
-            memcpy (&(event->device_service_id), uuid, 16);
+            array[i] = g_new0 (MbimEventEntry, 1);
+            memcpy (&(array[i]->device_service_id), uuid, 16);
             array_offset += 16;
 
-            event->cids_count = _mbim_message_read_guint32 (message, array_offset);
+            if (!_mbim_message_read_guint32 (message, array_offset, &(array[i])->cids_count, &inner_error))
+                break;
             array_offset += 4;
 
-            if (event->cids_count)
-                event->cids = _mbim_message_read_guint32_array (message, event->cids_count, array_offset);
-            else
-                event->cids = NULL;
-
-            array[i] = event;
+            if (array[i]->cids_count)
+                array[i]->cids = _mbim_message_read_guint32_array (message, array[i]->cids_count, array_offset);
             offset += 8;
         }
     }
diff --git a/src/libmbim-glib/mbim-proxy.c b/src/libmbim-glib/mbim-proxy.c
index 4001f8a..987d92d 100644
--- a/src/libmbim-glib/mbim-proxy.c
+++ b/src/libmbim-glib/mbim-proxy.c
@@ -802,7 +802,13 @@ process_internal_proxy_config (MbimProxy   *self,
     }
 
     /* Read requested timeout value */
-    request->timeout_secs = _mbim_message_read_guint32 (message, 8);
+    if (!_mbim_message_read_guint32 (message, 8, &request->timeout_secs, &error)) {
+        g_warning ("Error reading timeout from message: %s", error->message);
+        g_error_free (error);
+        request->response = build_proxy_control_command_done (message, MBIM_STATUS_ERROR_INVALID_PARAMETERS);
+        request_complete_and_free (request);
+        return TRUE;
+    }
 
     /* Check if some other client already handled the same device */
     device = peek_device_for_path (self, path);