Comment 8 for bug 11243

Message-ID: <email address hidden>
Date: Sun, 19 Dec 2004 13:29:40 +0100
From: Moritz Muehlenhoff <email address hidden>
To: <email address hidden>
Cc: <email address hidden>
Subject: CAN-2004-1154 proposed fix for woody

--8t9RHnE3ZwKMSgU+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,
the upstream patch against 3.0.9 wraps all memory allocations, strdup()s,
etc. into macros with boundary checks. Backporting this to 2.2.3 seems
way too intrusive and error-prone, so may I suggest the following fix:

According to the iDefense advisory it exploits an integer overflow in the
allocation of memory in source/rpc_parse/parse_sec.c:218:
If psa->num_aces is larger than 38347922, the value of 38347922+1 multiplied
with the value of ace[0] (112) is larger than an unsigned integer and
it wraps around.

So, as there is no way in regular Samba operation that a user requests
more than 38 million ACL entries the simplest and least intrusive way to
prevent this seems to be a simple sanity check that caps psa->num_aces:

if (psa->num_aces > 38347922)
 psa->num_aces = 38347922;

Does this seem like an acceptable solution? I haven't evaluated all possible
call flow that leads to the vulnerable code, though.

Cheers,
        Moritz

--8t9RHnE3ZwKMSgU+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="samba-can-2004-1154-proposed-fix.patch"

diff -Naur samba-2.2.3a.orig/source/rpc_parse/parse_sec.c samba-2.2.3a/source/rpc_parse/parse_sec.c
--- samba-2.2.3a.orig/source/rpc_parse/parse_sec.c 2002-02-03 01:46:50.000000000 +0100
+++ samba-2.2.3a/source/rpc_parse/parse_sec.c 2004-12-19 13:23:03.000000000 +0100
@@ -210,6 +210,10 @@
   return False;

  if (UNMARSHALLING(ps)) {
+ /* Prevent an integer overflow (CAN-2004-1154) */
+ if (psa->ace > 38347922)
+ psa->ace = 38347922;
+
   /*
    * Even if the num_aces is zero, allocate memory as there's a difference
    * between a non-present DACL (allow all access) and a DACL with no ACE's

--8t9RHnE3ZwKMSgU+--