Bug 5972 - Undefined Behavior in md5.c
Summary: Undefined Behavior in md5.c
Status: NEW
Alias: None
Product: uClibc
Classification: Unclassified
Component: Other (show other bugs)
Version: 0.9.33.3
Hardware: All Linux
: P5 normal
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-27 03:37 UTC by Jeffrey Walton
Modified: 2015-04-16 08:53 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jeffrey Walton 2013-02-27 03:37:35 UTC
$ grep -r -i rotate ./
./libcrypt/md5.c:/* ROTATE_LEFT rotates x left n bits. */
./libcrypt/md5.c:#define ROTATE_LEFT(x, n) (((x) << (n)) | ((x) >> (32-(n))))

I believe that's from OpenSSL. If you run that code with Clang and Regehr's Integer Overflow Checker (http://embed.cs.utah.edu/ioc/), you will find the rotate is outside the interval [0,31] inclusive. Specifically, the '32-n' when n is 0 means the shift is 32. I believe, but I'm not certain, the 'x << n' is OK.

'x >> (32-n)' violates standard C/C++, and the behavior is undefined (not implementation defined). See section 5.8 in http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2798.pdf.

Intel's compiler will remove the offending code if it can deduce its illegal, and I've seen it happen in the past. You can get a non-commercial copy for testing from http://software.intel.com/en-us/articles/non-commercial-software-download/.

I expect that GCC will eventually catch the illegal shifts too.
Comment 1 Jeffrey Walton 2013-02-27 20:52:52 UTC
My bad. That code appears to be from RSA Data Security.

/*
 * MD5C.C - RSA Data Security, Inc., MD5 message-digest algorithm
 *
 * Copyright (C) 1991-2, RSA Data Security, Inc. Created 1991. All
 * rights reserved.
 */
...