Bug 487251 (CVE-2009-0688)

Ok, I tracked down the problem with imtest.

The problem is that imtest is calling sasl_encode64() (from cyrus-sasl) to base64-encode the data before sending it to the server, but is telling sasl_encode64() that the buffer size into which it should place the encoded data is 2048 bytes long.

Look at the call to sasl_encode64() in auth_sasl() to see what I mean:

https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/cyrus/imtest/imtest.c?rev=HEAD

sasl_encode64() calculates that the encoded data would be greater than 2048 bytes, and returns SASL_BUFOVER.

The solution is to replace "2048" with "(unsigned) sizeof(inbase64)".

However, while investigating this problem I took a close look at sasl_encode64() and realized that the comments do not match the code. Specifically, the comments state that sasl_encode64() null-terminates the encoded data:

https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/sasl/lib/saslutil.c?rev=HEAD

However, this is only true if the size of the base64-encoded output is less than the size of the output buffer. If the size of the base64-encoded output would be greater than the size of the output buffer, sasl_encode64() returns SASL_BUFOVER, but if the size of the base64-encoded output is exactly the same size as the output buffer, the output buffer will NOT be null-terminated. This is easy to see by the tests at the beginning and end of the function:

    /* Will it fit? */
    olen = (inlen + 2) / 3 * 4;
    if (outlen)
      *outlen = olen;
    if (outmax < olen)
      return SASL_BUFOVER;

    ...

    if (olen < outmax)
      *out = '\0';

Now, if the fifth argument to sasl_encode64() is non-null, sasl_encode64 will write the size of the output it wrote to the location pointed to by the fifth argument. If the caller of sasl_encode64() then explicitly uses that size to read the contents of the output buffer, and never relies on (or calls a function that relies on) the output buffer being null-terminated, no buffer overflow is possible.

But when I realized that the comments describing sasl_encode64() were misleading, I scanned the rest of the Cyrus IMAP source code, and found multiple unsafe uses of sasl_encode64(); that is, instances where the caller of sasl_encode64() did not handle the case where sasl_encode64() would not null-terminate its output, and then performed subsequent operations (e.g., str* functions) that assume their input is always null-terminated. I found those unsafe uses of sasl_encode64() solely in the cyrus-imap package; I didn't bother to look in any other packages.

Given that there may be many more vendors/packages whose authors were misled into using sasl_encode64() unsafely, I have also reported this to CERT. The CERT report tracking ID is VRF#FTACZUPV.

(I have NOT updated the Andrew Bugzilla bug report, because that bug report is world-readable.)

It seems there is one buggy usage of sasl_encode64 in cyrus-sasl itself as well. The problem is in create_nonce() in plugins/digestmd5.c. The function always passes a buffer of exactly the required length so it is never null-terminated, but the buffer is later handled as a null-terminated string.

Also we must note that the callers will have to be fixed if they are wrong and not the sasl_encode64 itself. Fixing sasl_encode64() - that is returning the error if the buffer does not contain space for the nul terminator - would break ABI.

Unfortunately, yes, it would; any caller that knew how much space would be required for base64 encoding and allocated a buffer of exactly that size would break if sasl_encode64() were changed to always null-terminate its output, because sasl_encode64() would then fail with SASL_BUFOVER.

As Tomas noted in comment 3, this is exactly what create_nonce() does.

Created attachment 341654 [details]
Upstream patch to fix the issue

I don't think there is any way to fix it "cleverly" without breaking ABI. So the callers will have to be found and fixed (if there are any broken ones).

BTW I was mistaken in the comment #3 as the encode64() call in create_nonce() in digestmd5.c is OK because there is different rounding in the create_nonce() than in the encode64() which fortunately for the nonce size always produces buffer large enough to hold the terminating '\0'.

Already public via CERT VU#238019 advisory (lifting embargo):

The Cyrus SASL library contains a buffer overflow vulnerability
that could allow an attacker to execute code or cause a vulnerable
program to crash. The sasl_encode64() function converts a string
into base64. The Cyrus SASL library contains buffer overflows that
occur because of unsafe use of the sasl_encode64() function.
A remote attacker might be able to execute code, or cause any programs
relying on SASL to crash or be unavailable.

References:
http://www.kb.cert.org/vuls/id/238019
http://www.kb.cert.org/vuls/id/RGII-7RYLZQ
ftp://ftp.andrew.cmu.edu/pub/cyrus-mail/cyrus-sasl-2.1.23.tar.gz
http://en.wikipedia.org/w/index.php?title=Base64&oldid=285664115

Credit: James Ralston

CVE-2009-0688 entry:

Multiple buffer overflows in the CMU Cyrus SASL library before 2.1.23
might allow remote attackers to execute arbitrary code or cause a
denial of service (application crash) via strings that are used as
input to the sasl_encode64 function in lib/saslutil.c. 

References:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-0688
ftp://ftp.andrew.cmu.edu/pub/cyrus-mail/cyrus-sasl-2.1.23.tar.gz
http://www.kb.cert.org/vuls/id/238019
http://www.securityfocus.com/bid/34961

Cyrus vendor stamenet regarding the patch and potential fix
(from http://www.kb.cert.org/vuls/id/RGII-7RYLZQ):

While this patch will fix currently vulnerable code, it can cause non-vulnerable existing code to break. Here's a function prototype
from include/saslutil.h to clarify my explanation:

/* base64 encode
* in -- input data
* inlen -- input data length
* out -- output buffer (will be NUL terminated)
* outmax -- max size of output buffer
* result:
* outlen -- gets actual length of output buffer (optional)
*
* Returns SASL_OK on success, SASL_BUFOVER if result won't fit
*/
LIBSASL_API int sasl_encode64(const char *in, unsigned inlen,
char *out, unsigned outmax, unsigned *outlen);

Assume a scenario where calling code has been written in such a way
that it calculates the exact size required for base64 encoding in
advance, then allocates a buffer of that exact size, passing
a pointer to the buffer into sasl_encode64() as *out. As long as
this code does not anticipate that the buffer is NUL-terminated
(does not call any string-handling functions like strlen(),
for example) the code will work and it will not be vulnerable.

Once this patch is applied, that same code will break because
sasl_encode64() will begin to return SASL_BUFOVER.

I did some investigation into this today. Here is what I found

The things in RHEL that use cyrus-sasl:

cyrus-sasl - tmraz says this was checked and is OK

postfix - Enough space is allocated in the RHEL version

cyrus-imapd - This one just allocates a large buffer, we need to see if that works as expected.

mutt - mutt does some clever things with the string once it's used, it might be a problem. Given it's using a heap variable, an info leak is going to be very unlikely, a crash will be the expected outcome (I would not call this a security flaw)

inn - It allocates buffer * 2, which is more than needed.

sendmail - Sendmail has some questionable uses in sendmail/usersmtp.c, the same issue as cyrus-imapd above.


Both in the sendmail and syrus-imapd cases, the strings in question are not used for anything odd, they are simply copied elsewhere. That means that we're going to see a crash, leaking some memory to the server, or auth failures.

As the reporter noticed the issue in cyrus-imapd, that obviously needs to be fixed.

cyrus-imapd and sendmail case are further tracked via separate bugs:

- sendmail - bug #504186 - further investigation showed all sasl_encode64 uses are safe, no update needed

- cyrus-imapd - bug #504207 - updates for Red Hat Enterprise Linux 4 and 5 were released via RHSA-2009:1116

Due to the fact that the upstream fix for Cyrus SASL introduces a significant enough change that it would break ABI compatibility, the Cyrus SASL library itself will not be updated.  However, we have investigated a number of applications that use the sasl_encode64() function to ensure they use it correctly.  As a result of this investigation, Cyrus IMAP has been updated to correct the flaw (bug #504207, released as RHSA-2009:1116).  With this Cyrus IMAP update, we believe all programs that use this function from Cyrus SASL incorrectly have been fixed, and as such this is no longer an issue for Red Hat Enterprise Linux.

Is there a way to safely update the current Cyrus SASL 2.1.22 to 2.1.23 on RHEL 5.3? There are numerous dependancies on the 2.1.22 that we are afraid will break the system if we update to 2.1.23.

E_eye Retina Scanner still report this as Major, Category I vulnerability during scanning as of Aug/18/2009.

The only difference between 2.1.22 and 2.1.23 is the CVE-2009-0688 workaround same as is in RHEL-5. Replacing RHEL-5's cyrus-sasl by the vanila 2.1.23 may lead some bugs into system, because the RHEL-5's one have some other bugs fixed.