[PATCH] fix unsafe macro calls and questionable casts in libcrypt/md5.c

[Denys Vlasenko]

Hi,

Just noticed this:

#define F(x, y, z) (((x) & (y)) | ((~x) & (z)))
...
#define FF(a, b, c, d, x, s, ac) { \
        (a) += F ((b), (c), (d)) + (x) + (u_int32_t)(ac); \
        (a) = ROTATE_LEFT ((a), (s)); \
        (a) += (b); \
        }
...
...
        const char *pp;
...
        for ( i = 0 ; i < 16 ; i++ ) {
                FF (a, b, c, d, x[(int)(*pp++)], ps[i&0x3], *pc++);
                temp = d; d = c; c = b; b = a; a = temp;
        }

The invocation of FF macro looks bad.

x[(int)(*pp++)] has a side effect (pp is incremented).
It's dangerous to use such expressions as macro args.
By luck, here it so happens that 5th argument
is not evaluated twice by the macro
(for example, 1st and 2nd are evaluated multiple times).

Cast (int)(*pp) is strange. Was it intended by author
to make it work properly if char is a signed type?
Did he meant (unsigned) cast? But even if so,
it is not working: char -> int promotion happens *before*
the cast:

# cat t.c
int main() {
        signed char pp[] = { 0xff };
        if ((int)(*pp) < 0) printf("boo!\n");
        if ((unsigned)(*pp) > 0x100) printf("BOO!\n");
}
# gcc t.c
t.c: In function 'main':
t.c:3: warning: incompatible implicit declaration of built-in function 'printf'
# ./a.out
boo!
BOO!

The attached patch fixes these things. Please review.

Object code difference is just these two instructions:

@@ -156,14 +156,14 @@
 31 f8                  xor    %edi,%eax
 8d 14 08               lea    (%eax,%ecx,1),%edx
 8b 4c 24 24            mov    0x24(%esp),%ecx
-0f be 01               movsbl (%ecx),%eax
+0f b6 01               movzbl (%ecx),%eax
 03 54 84 2c            add    0x2c(%esp,%eax,4),%edx
 8b 44 24 20            mov    0x20(%esp),%eax
 03 10                  add    (%eax),%edx
 8b 4c 24 28            mov    0x28(%esp),%ecx
 83 e1 03               and    $0x3,%ecx
 8b 44 24 1c            mov    0x1c(%esp),%eax
-0f be 0c 08            movsbl (%eax,%ecx,1),%ecx
+0f b6 0c 08            movzbl (%eax,%ecx,1),%ecx
 d3 c2                  rol    %cl,%edx
 01 f2                  add    %esi,%edx
 ff 44 24 28            incl   0x28(%esp)

--
vda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6.patch
Type: text/x-diff
Size: 5109 bytes
Desc: not available
Url : http://busybox.net/lists/uclibc/attachments/20080612/6fb20a15/attachment.patch