From dfaee1e78ee285535dd337638d8b2b784a42db77 Mon Sep 17 00:00:00 2001
From: Aleksander Morgado <aleksandermj@chromium.org>
Date: Mon, 31 Oct 2022 13:21:32 +0000
Subject: [PATCH] libmbim-glib,message: fix validation of complete fragment
For messages that may be composed of multiple fragments, the
_mbim_message_validate_type_header() method would validate wether the
fragment header can be read or not, because not all fragments contain
the additional type-specific header contents.
But once the message is complete with all fragments, the message
validation must also ensure that the type-specific header contets are
readable before attempting to read them, or we will end up with
invalid memory reads.
Detected via ASAN+Fuzzing:
==5169==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030000bc9ac at pc 0x55a9fc0d536d bp 0x7ffc556bb7b0 sp 0x7ffc556bb7a8
READ of size 4 at 0x6030000bc9ac thread T0
#0 0x55a9fc0d536c in _mbim_message_validate_complete_fragment libmbim-9999-build/../libmbim-9999/src/libmbim-glib/mbim-message.c:239:28
#1 0x55a9fc0baf40 in _mbim_message_validate_fragment libmbim-9999-build/../libmbim-9999/src/libmbim-glib/mbim-message.c:279:12
#2 0x55a9fc0ba7a1 in mbim_message_validate libmbim-9999-build/../libmbim-9999/src/libmbim-glib/mbim-message.c:292:12
#3 0x55a9fc0b9af1 in LLVMFuzzerTestOneInput libmbim-9999-build/../libmbim-9999/src/libmbim-glib/test/test-message-fuzzer.c:25:5
(cherry picked from commit 37825b4ecbffdf94eb3af935ce7e8032be63f99a)
---
src/libmbim-glib/mbim-message.c | 37 ++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/src/libmbim-glib/mbim-message.c b/src/libmbim-glib/mbim-message.c
index 68780439..1b9975d1 100644
--- a/src/libmbim-glib/mbim-message.c
+++ b/src/libmbim-glib/mbim-message.c
@@ -167,7 +167,10 @@ _mbim_message_validate_type_header (const MbimMessage *self,
return FALSE;
/* Validate message type and get additional minimum required size based on
- * message type */
+ * message type. At this stage, we only check the fragment header validity for
+ * Command, Command Done and Indication messages, because only the 'complete'
+ * fragment will have the additional header contents specific to each message
+ * type. */
switch (MBIM_MESSAGE_GET_MESSAGE_TYPE (self)) {
case MBIM_MESSAGE_TYPE_OPEN:
message_header_size = sizeof (struct header) + sizeof (struct open_message);
@@ -231,6 +234,38 @@ _mbim_message_validate_complete_fragment (const MbimMessage *self,
GError **error)
{
gsize message_size = 0;
+ gsize message_header_size = 0;
+
+ /* Before reading the information buffer length we must validate that the
+ * message type header is readable. */
+ switch (MBIM_MESSAGE_GET_MESSAGE_TYPE (self)) {
+ case MBIM_MESSAGE_TYPE_COMMAND:
+ message_header_size = sizeof (struct header) + sizeof (struct command_message);
+ break;
+ case MBIM_MESSAGE_TYPE_COMMAND_DONE:
+ message_header_size = sizeof (struct header) + sizeof (struct command_done_message);
+ break;
+ case MBIM_MESSAGE_TYPE_INDICATE_STATUS:
+ message_header_size = sizeof (struct header) + sizeof (struct indicate_status_message);
+ break;
+ case MBIM_MESSAGE_TYPE_OPEN:
+ case MBIM_MESSAGE_TYPE_CLOSE:
+ case MBIM_MESSAGE_TYPE_HOST_ERROR:
+ case MBIM_MESSAGE_TYPE_OPEN_DONE:
+ case MBIM_MESSAGE_TYPE_CLOSE_DONE:
+ case MBIM_MESSAGE_TYPE_FUNCTION_ERROR:
+ case MBIM_MESSAGE_TYPE_INVALID:
+ default:
+ g_assert_not_reached ();
+ break;
+ }
+
+ /* Validate that the message type specific header can be read. */
+ if (message_header_size && (MBIM_MESSAGE_GET_MESSAGE_LENGTH (self) < message_header_size)) {
+ g_set_error (error, MBIM_CORE_ERROR, MBIM_CORE_ERROR_INVALID_MESSAGE,
+ "Invalid message size: fragment type header incomplete");
+ return FALSE;
+ }
/* Get information buffer size */
switch (MBIM_MESSAGE_GET_MESSAGE_TYPE (self)) {
--
GitLab