From ef2fe73e813cddfb52bbd456f2a33b2237428240 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Tue, 8 Nov 2022 12:56:14 +0000 Subject: libqmi-glib,message: check message header size before accessing it Reported by asan during fuzzing: ==1==WARNING: MemorySanitizer: use-of-uninitialized-value 0x7f4261ef6c57 in message_check /build/amd64-generic/tmp/portage/net-libs/libqmi-1.32.0-r125/work/libqmi-1.32.0/src/libqmi-glib/qmi-message.c:331:9 0x7f4261f00b9f in qmi_message_new_from_raw /build/amd64-generic/tmp/portage/net-libs/libqmi-1.32.0-r125/work/libqmi-1.32.0/src/libqmi-glib/qmi-message.c:1527:10 0x557c9f672e6d in LLVMFuzzerTestOneInput /build/amd64-generic/tmp/portage/net-libs/libqmi-1.32.0-r125/work/libqmi-1.32.0/src/libqmi-glib/test/test-message-fuzzer.c:26:15 0x557c9f5c1792 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) 0x557c9f5ad8a3 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) 0x557c9f5b2b64 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) 0x557c9f5da4f2 in main 0x7f42617686c5 in __libc_start_call_main 0x7f4261768781 in __libc_start_main_impl 0x557c9f5a4940 in _start --- src/libqmi-glib/qmi-message.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/libqmi-glib/qmi-message.c b/src/libqmi-glib/qmi-message.c index 3567342e..cdb49ef2 100644 --- a/src/libqmi-glib/qmi-message.c +++ b/src/libqmi-glib/qmi-message.c @@ -311,29 +311,30 @@ qmi_tlv_next (QmiMessage *self, * Returns: %TRUE if the message is valid, %FALSE otherwise. */ static gboolean -message_check (QmiMessage *self, - GError **error) +message_check (QmiMessage *self, + GError **error) { - gsize header_length; - guint8 *end; + gsize header_length; + gsize qmux_length; + guint8 *end; struct tlv *tlv; - if (((struct full_message *)(self->data))->marker != QMI_MESSAGE_QMUX_MARKER) { + if (self->len < (1 + sizeof (struct qmux))) { g_set_error (error, QMI_CORE_ERROR, QMI_CORE_ERROR_INVALID_MESSAGE, - "Marker is incorrect (0x%02x != 0x%02x)", - ((struct full_message *)(self->data))->marker, - QMI_MESSAGE_QMUX_MARKER); + "QMUX length too short for QMUX header (%u < %" G_GSIZE_FORMAT ")", + get_qmux_length (self), sizeof (struct qmux)); return FALSE; } - if (get_qmux_length (self) < sizeof (struct qmux)) { + if (((struct full_message *)(self->data))->marker != QMI_MESSAGE_QMUX_MARKER) { g_set_error (error, QMI_CORE_ERROR, QMI_CORE_ERROR_INVALID_MESSAGE, - "QMUX length too short for QMUX header (%u < %" G_GSIZE_FORMAT ")", - get_qmux_length (self), sizeof (struct qmux)); + "Marker is incorrect (0x%02x != 0x%02x)", + ((struct full_message *)(self->data))->marker, + QMI_MESSAGE_QMUX_MARKER); return FALSE; } @@ -341,7 +342,8 @@ message_check (QmiMessage *self, * qmux length is one byte shorter than buffer length because qmux * length does not include the qmux frame marker. */ - if (get_qmux_length (self) != self->len - 1) { + qmux_length = get_qmux_length (self); + if (qmux_length != self->len - 1) { g_set_error (error, QMI_CORE_ERROR, QMI_CORE_ERROR_INVALID_MESSAGE, @@ -354,7 +356,7 @@ message_check (QmiMessage *self, sizeof (struct control_header) : sizeof (struct service_header)); - if (get_qmux_length (self) < header_length) { + if (qmux_length < header_length) { g_set_error (error, QMI_CORE_ERROR, QMI_CORE_ERROR_INVALID_MESSAGE, @@ -363,7 +365,7 @@ message_check (QmiMessage *self, return FALSE; } - if (get_qmux_length (self) - header_length != get_all_tlvs_length (self)) { + if (qmux_length - header_length != get_all_tlvs_length (self)) { g_set_error (error, QMI_CORE_ERROR, QMI_CORE_ERROR_INVALID_MESSAGE, -- cgit v1.2.1