Re: md5 broken?

[Jim Meyering <jim@meyering.net>]

Antoine Levitt wrote:
...
> (md5 "truc")
> => 45723a2af3788c4ff17f8d1114760e62
> (which is the same thing as md5sum)
>
>>From an emacs just compiled,
>
> (md5 "truc")
> => 45723a2aff78ff4fff7fff1114760e62
> (it seems some digits have been randomly replaced by f, for some reason)
...

Thanks for the report.
That was due to yesterday's crypto_hash_function change.
It switched from unsigned to signed char pointers.
The patch below fixes it by introducing the tiny "to_uchar" function
from coreutils/src/system.h.  It's safer to use a tiny helper
function like that rather than a cast.

I fixed it with this:

2011-05-28  Jim Meyering  <meyering@redhat.com>

	avoid a sign-extension bug in crypto_hash_function
	* fns.c (to_uchar): Define.
	(crypto_hash_function): Use it to convert some newly-signed
	variables to unsigned, to avoid sign-extension bugs.  For example,
	without this change, (md5 "truc") would evaluate to
	45723a2aff78ff4fff7fff1114760e62 rather than the expected
	45723a2af3788c4ff17f8d1114760e62.  Reported by Antoine Levitt in
	http://thread.gmane.org/gmane.emacs.devel/139824


=== modified file 'src/fns.c'
--- src/fns.c	2011-05-27 19:37:32 +0000
+++ src/fns.c	2011-05-28 12:09:59 +0000
@@ -4520,6 +4520,11 @@
 #include "md5.h"
 #include "sha1.h"

+/* Convert a possibly-signed character to an unsigned character.  This is
+   a bit safer than casting to unsigned char, since it catches some type
+   errors that the cast doesn't.  */
+static inline unsigned char to_uchar (char ch) { return ch; }
+
 /* TYPE: 0 for md5, 1 for sha1. */

 static Lisp_Object
@@ -4717,7 +4722,7 @@
 	  {
 	    char value[33];
 	    for (i = 0; i < 16; i++)
-	      sprintf (&value[2 * i], "%02x", digest[i]);
+	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
 	    res = make_string (value, 32);
 	  }
 	else
@@ -4735,7 +4740,7 @@
 	  {
 	    char value[41];
 	    for (i = 0; i < 20; i++)
-	      sprintf (&value[2 * i], "%02x", digest[i]);
+	      sprintf (&value[2 * i], "%02x", to_uchar (digest[i]));
 	    res = make_string (value, 40);
 	  }
 	else