Bug 5972

Summary:	Undefined Behavior in md5.c
Product:	uClibc	Reporter:	Jeffrey Walton <noloader>
Component:	Other	Assignee:	unassigned
Status:	NEW ---
Severity:	normal	CC:	buildroot, uclibc-cvs
Priority:	P5
Version:	0.9.33.3
Target Milestone:	---
Hardware:	All
OS:	Linux
Host:		Target:
Build:

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.
 */
...