xorl %eax, %eax

CVE-2010-3069: SAMBA SID Remote Stack Buffer Overflow

with 3 comments

You probably have already heard of this bug recently disclosed. It was reported by Andrew Bartlett and it affects releases prior to 3.5.5 as you can read in the official release notes. Here is the buggy code as seen at source3/lib/util_sid.c of the 3.5.4 release of the SAMBA.

/*****************************************************************
 Parse a on-the-wire SID to a DOM_SID.
*****************************************************************/  

bool sid_parse(const char *inbuf, size_t len, DOM_SID *sid)
{
        int i;
        if (len < 8)
                return False;

        ZERO_STRUCTP(sid);

        sid->sid_rev_num = CVAL(inbuf, 0);
        sid->num_auths = CVAL(inbuf, 1);
        memcpy(sid->id_auth, inbuf+2, 6);
        if (len < 8 + sid->num_auths*4)
                return False;
        for (i=0;i<sid->num_auths;i++)
                sid->sub_auths[i] = IVAL(inbuf, 8+i*4);
        return True;
}

In order to have a better understanding of the bug here you need to know the ‘DOM_SID’ data type which can be found at source3/include/smb.h like this:

/**
 * @brief Security Identifier
 *
 * @sa http://msdn.microsoft.com/library/default.asp?url=/library/en-us/security/accctrl_38yn.asp
 **/
typedef struct dom_sid DOM_SID;

And of course, ‘dom_sid’ structure is defined at librpc/gen_ndr/security.h:

struct dom_sid {
        uint8_t sid_rev_num;
        int8_t num_auths;/* [range(0,15)] */
        uint8_t id_auth[6];
        uint32_t sub_auths[15];
}/* [noprint,gensize,nopull,public,nopush,nosize] */;

Back to sid_parse(), you can now see that it checks that length is no more than 8 and then uses ZERO_STRUCTP() (located at lib/util/memory.h) to zero out ‘sid’ like this:

#define ZERO_STRUCTP(x) do { if ((x) != NULL) memset((char *)(x), 0, sizeof(*(x))); } while(0)

Then, it will initialize ‘sid->sid_rev_num’ (representing the SID revision number) and the ‘sid->num_auths’ (which is the number of sub-authorities) from the input buffer ‘inbuf’ using CVAL() macro from lib/util/byteorder.h to retrieve the first and second values respectively.

#define CVAL(buf,pos) (((uint8_t *)(buf))[pos])

The next code is a call to memcpy(3) to copy the following 6 Bytes to the ‘sid->id_auth’ (which is the identifier authority). When this is completed, it will check that length is no less than the number of sub-authorities multiplied by 4 (the size of each one) plus the 8 Bytes. Assuming that everything is fine, it will enter a ‘for’ loop for the number of sub-authorities to copy them in the ‘sid->sub_auths[]’ array from the input buffer.
The bug here is that ‘num_auths’ even though it should only support numbers ranging from 0 to 15, it’s a signed 8-Bit long integer meaning that it can store values from -127 up to 127. Because of this, a malicious user could send a specially crafted SID header that will result in an integer overflow during the multiplication inside the ‘if’ clause and later on, a stack based buffer overflow inside the ‘for’ loop.
To fix this, a new constant was defined in libcli/security/dom_sid.h:

 char *dom_sid_string(TALLOC_CTX *mem_ctx, const struct dom_sid *sid);
 
+#ifndef MAXSUBAUTHS
+#define MAXSUBAUTHS 15 /* max sub authorities in a SID */
+#endif
+
 #endif /*_DOM_SID_H_*/

Which is used to set the maximum number of sub-authorities in a security ID and it was used in the vulnerable function like this:

 	sid->num_auths = CVAL(inbuf, 1);
+	if (sid->num_auths > MAXSUBAUTHS) {
+		return false;
+	}
 	memcpy(sid->id_auth, inbuf+2, 6);

Also, dump_sid(), parse_user_quota_record(), call_nt_transact_ioctl(), call_nt_transact_get_user_quota() and call_nt_transact_set_user_quota() were updated to check the return value of sid_parse().
Regarding the exploitation of the vulnerability the only publicly available information is H. D. Moore’s tweet:

exploiting the Samba vulnerability 
does require quota support AND a 
valid admin user/pass, nothing to 
see here, move along :)

Written by xorl

September 21, 2010 at 22:42

Posted in bugs

3 Responses

Subscribe to comments with RSS.

  1. Yep, sad case. It was confirmed privately by @legerov as well. Everyone all spun up over a “critical” bug that boils down to admin to root…

    jduck

    September 21, 2010 at 22:59

  2. I don’t understand your description. I thought that an integer overflow is not possible here because these are only int8 values and an upcast (up to int32) will happen in the comparison, and 4*(+/-)127 +8 can’t overflow the max int32 value.

    Otoh len also comes from the input, so the check against it seems useless. It appears to me that if len was larger then 15*4 +8, num_auths could be larger then 15, thus you get a BOF.

    Can you explain how I’m wrong? Thanks!!

    pvppyk1tten

    October 2, 2010 at 00:30

  3. @pvppyk1tten:
    1) Regarding the type promotion you’re probably right but I haven’t attempted to exploit this bug to be sure. However, You’re probably right.
    2) ‘len’ value is not entirely user controlled. I don’t think that you can do anything with it in this case.

    xorl

    October 4, 2010 at 15:01


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s