xorl %eax, %eax

CVE-2009-0688: CMU Cyrus SASL off-by-one Overflow

with 3 comments

This vulnerability was reported by James Ralston and publicly disclosed on 14 May 2009. It affects CMU Cyrus SASL library prior to 2.1.23 release. CMU Cyrus SASL is a popular authentication library which is used in numerous operating systems including various Linux distributions such as Ubuntu, Fedora, Debian etc. as well as commercial operating systems such as Sun Solaris, SCO UnixWare etc. Here is the vulnerable function from 2.1.22 release of CMU Cyrus SASL library:

2  * Rob Siemborski
3  * Tim Martin
4  * $Id: saslutil.c,v 1.44 2006/03/13 18:26:36 mel Exp $
5  */
6 /*
7  * Copyright (c) 1998-2003 Carnegie Mellon University.  All rights reserved.
 ...
103 /* base64 encode
104  *  in      -- input data
105  *  inlen   -- input data length
106  *  out     -- output buffer (will be NUL terminated)
107  *  outmax  -- max size of output buffer
108  * result:
109  *  outlen  -- gets actual length of output buffer (optional)
110  *
111  * Returns SASL_OK on success, SASL_BUFOVER if result won't fit
112  */
113
114 int sasl_encode64(const char *_in, unsigned inlen,
115                   char *_out, unsigned outmax, unsigned *outlen)
116 {
117     const unsigned char *in = (const unsigned char *)_in;
118     unsigned char *out = (unsigned char *)_out;
119     unsigned char oval;
120     char *blah;
121     unsigned olen;
 ...
126     /* Will it fit? */
127     olen = (inlen + 2) / 3 * 4;
128     if (outlen)
129       *outlen = olen;
130     if (outmax < olen)
131       return SASL_BUFOVER;
 ...
154     if (olen < outmax)
155       *out = '';
156
157     return SASL_OK;
158 }


This routine from lib/saslutil.c is widely used for Base64 encoding wherever this is required. The problem with the above code is that the calculated output length (olen at line 127) can be exactly the same as outmax and this can lead to an off-by-one overwrite at line 155 where the NULL termination takes place. To patch this the following fix was applied:

 olen = (inlen + 2) / 3 * 4;
-    if (outlen)
+    if (outlen) {
 *outlen = olen;
-    if (outmax < olen)
+    }
+    if (outmax <= olen) {
 return SASL_BUFOVER;
+    }


This way it checks for output lengths exactly the same as the maximum output length. Using this, the NULL termination check at line 154 is no longer needed, so it was removed from the new release:

-    if (olen < outmax)
-      *out = '';
+    *out = '';

Written by xorl

May 18, 2009 at 21:06

Posted in vulnerabilities

3 Responses

Subscribe to comments with RSS.

  1. But… There is check before code which can overflow:

    154 if (olen < outmax)
    155 *out = '';

    So if olen == outmax then:

    155 *out = '';

    won't be called. So there is not off-by-one. Isn't it?

    test_xxx

    May 25, 2009 at 23:51

  2. From my snippet you’re right. But by having a quick look at sasl_encode64() you can see that ‘out’ gets overwritten before reaching line 154 where this check takes place. Here is the operations being performed at lines 132-153:

        /* Do the work... */
        blah=(char *) out;
        while (inlen >= 3) {
          /* user provided max buffer size; make sure we don't go over it */
            *out++ = basis_64[in[0] >> 2];
            *out++ = basis_64[((in[0] <> 4)];
            *out++ = basis_64[((in[1] <> 6)];
            *out++ = basis_64[in[2] & 0x3f];
            in += 3;
            inlen -= 3;
        }
        if (inlen > 0) {
          /* user provided max buffer size; make sure we don't go over it */
            *out++ = basis_64[in[0] >> 2];
            oval = (in[0] < 1) oval |= in[1] >> 4;
            *out++ = basis_64[oval];
            *out++ = (inlen < 2) ? '=' : basis_64[(in[1] << 2) & 0x3c];
            *out++ = '=';
        }
    

    It was my mistake not mentioning this in the initial post. What ‘out’ at line 155 points to, is the ‘=’ you can see in the above snippet.

    xorl

    May 26, 2009 at 16:44

  3. Ah.. ok :) Thanks…

    Btw. You have great blog… Good work!

    test_xxx

    June 27, 2009 at 11:14


Leave a comment