File bb15d481-Fix-flexible-array-buffer-overflow.patch of Package spice-gtk

Overview Repositories Revisions Requests Users Attributes Meta

File bb15d481-Fix-flexible-array-buffer-overflow.patch of Package spice-gtk (Revision bcdeb29c24a3d0fd791648278f1448c2)

Currently displaying revision bcdeb29c24a3d0fd791648278f1448c2 , Show latest

From bb15d4815ab586b4c4a20f4a565970a44824c42c Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
Date: Fri, 18 May 2018 11:41:57 +0100
Subject: [PATCH] Fix flexible array buffer overflow

This is kind of a DoS, possibly flexible array in the protocol
causes the network size check to be ignored due to integer overflows.

The size of flexible array is computed as (message_end - position),
then this size is added to the number of bytes before the array and
this number is used to check if we overflow initial message.

An example is:

message {
        uint32 dummy[2];
        uint8 data[] @end;
    } LenMessage;

which generated this (simplified remove useless code) code:

{ /* data */
        data__nelements = message_end - (start + 8);

data__nw_size = data__nelements;
    }

nw_size = 8 + data__nw_size;

/* Check if message fits in reported side */
    if (nw_size > (uintptr_t) (message_end - start)) {
        return NULL;
    }

Following code:
- data__nelements == message_end - (start + 8)
- data__nw_size == data__nelements == message_end - (start + 8)
- nw_size == 8 + data__nw_size == 8 + message_end - (start + 8) ==
  8 + message_end - start - 8 == message_end -start
- the check for overflow is (nw_size > (message_end - start)) but
  nw_size == message_end - start so the check is doing
  ((message_end - start) > (message_end - start)) which is always false.

If message_end - start < 8 then data__nelements (number of element
on the array above) computation generate an integer underflow that
later create a buffer overflow.

Add a check to make sure that the array starts before the message ends
to avoid the overflow.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
---
 python_modules/demarshal.py  | 1 +
 tests/test-marshallers.c     | 8 ++++++++
 tests/test-marshallers.h     | 5 +++++
 tests/test-marshallers.proto | 5 +++++
 4 files changed, 19 insertions(+)

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 7b53361..5a237a6 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -331,6 +331,7 @@ def write_validate_array_item(writer, container, item, scope, parent_scope, star
         writer.assign(nelements, array.size)
     elif array.is_remaining_length():
         if element_type.is_fixed_nw_size():
+            writer.error_check("%s > message_end" % item.get_position())
             if element_type.get_fixed_nw_size() == 1:
                 writer.assign(nelements, "message_end - %s" % item.get_position())
             else:
-- 
2.18.0

Places

File bb15d481-Fix-flexible-array-buffer-overflow.patch of Package spice-gtk (Revision bcdeb29c24a3d0fd791648278f1448c2)

Places