On Wed, Dec 09, 2009 at 10:08:38PM +0000, Jochen Voss wrote:
> while experimenting with coccinelle, I accidentially found what I
Good work, thanks! Can you share your spatches? For example memset()
bugs are pretty common. It'd be nice to have somewhere on internet
a collection of spatches for generic bugs.
> believe is a bug in util-linux-ng release 2.17-rc2 (downloaded today).
> The probem is the following code in lib/md5.c (around line 153):
>
> void MD5Final(unsigned char digest[16], struct MD5Context *ctx)
> {
> [...]
> memset(ctx, 0, sizeof(ctx)); /* In case it's sensitive */
> }
>
> The third argument of memset should probably be the size of 'struct
> MD5Context' instead of the size of the pointer. So my guess is
> that the memset line should be
>
> memset(ctx, 0, sizeof(*ctx)); /* In case it's sensitive */
>
> instead. I don't know whether this actually causes a problem,
> but the comment makes it seem possible that it does.
The memset() does not have any impact to the MD5 hashes. From my
point of view it seems like a paranoid operation for a case that you
have any security sensitive data in the MD5Context buffers or so.
I have applied the patch below.
Karel
>From 6596057175c6ed342dc20e85eae8a42eb29b629f Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@xxxxxxxxxx>
Date: Thu, 10 Dec 2009 11:59:46 +0100
Subject: [PATCH] lib: bug (typo) in function MD5Final()
On Wed, Dec 09, 2009 at 10:08:38PM +0000, Jochen Voss wrote:
> while experimenting with coccinelle, I accidentally found what I
> believe is a bug in util-linux-ng release 2.17-rc2 (downloaded
> today). The problem is the following code in lib/md5.c (around line
> 153):
>
> void MD5Final(unsigned char digest[16], struct MD5Context *ctx)
> {
> [...]
> memset(ctx, 0, sizeof(ctx)); /* In case it's sensitive */
> }
>
> The third argument of memset should probably be the size of 'struct
> MD5Context' instead of the size of the pointer. So my guess is
> that the memset line should be
>
> memset(ctx, 0, sizeof(*ctx)); /* In case it's sensitive */
>
> instead. I don't know whether this actually causes a problem,
> but the comment makes it seem possible that it does.
Note, this typo does not have any impact on the utils in the
util-linux-ng project, because we don't use MD5 for any security
sensitive data or cryptographic stuff. The typo also does not have any
impact to the final MD5 hashes.
Reported-by: Jochen Voss <voss@xxxxxxxxxx>
Signed-off-by: Karel Zak <kzak@xxxxxxxxxx>
---
lib/md5.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/md5.c b/lib/md5.c
index 3069845..6ad4b68 100644
--- a/lib/md5.c
+++ b/lib/md5.c
@@ -150,7 +150,7 @@ void MD5Final(unsigned char digest[16], struct MD5Context *ctx)
MD5Transform(ctx->buf, (uint32_t *) ctx->in);
byteReverse((unsigned char *) ctx->buf, 4);
memcpy(digest, ctx->buf, 16);
- memset(ctx, 0, sizeof(ctx)); /* In case it's sensitive */
+ memset(ctx, 0, sizeof(*ctx)); /* In case it's sensitive */
}
#ifndef ASM_MD5
--
1.6.5.2
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
