git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicolas Pitre <nico@cam.org>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: [PATCH] fix signed range problems with hex conversions
Date: Wed, 30 May 2007 13:09:24 -0400 (EDT)	[thread overview]
Message-ID: <alpine.LFD.0.99.0705301213450.11491@xanadu.home> (raw)

The get_sha1_hex() function is defined as accepting a char array.  
Because the char type is signed by default on many architectures, 
get_sha1_hex() can be passed a pointer to negative chars. This can 
happen with user input containing chars with the top bit set.

Then those chars are passed to hexval() which is defined as accepting an 
unsigned int value. Whenever a signed char is promoted to an int, the 
promotion is always signed and then the result is stored in the unsigned 
int variable.  In the negative char case that means really large 
unsigned int values will result, and then the hexval_table is happily 
indexed with that value.

On 32-bit architectures the large int value will create a wrap-around 
and a byte located somewhere before the hexval_table array in memory 
will be fetched.  Depending on that byte value a bogus SHA1 value could 
be returned.

On 64-bit architectures the large int value will most probably cause a 
segmentation fault.

This patch adds a range test to hexval() in order to prevent this.  Also 
let's index the hexval_table array directly in get_sha1_hex() using 
explicitly unsigned chars to avoid the range test producing faster
code.

While at it, make hexval_table const.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

diff --git a/cache.h b/cache.h
index f675223..30fcaa9 100644
--- a/cache.h
+++ b/cache.h
@@ -359,10 +359,10 @@ extern void *map_sha1_file(const unsigned char *sha1, unsigned long *);
 extern int has_pack_file(const unsigned char *sha1);
 extern int has_pack_index(const unsigned char *sha1);
 
-extern signed char hexval_table[256];
+extern const signed char hexval_table[256];
 static inline unsigned int hexval(unsigned int c)
 {
-	return hexval_table[c];
+	return (c & ~0xff) ? -1 : hexval_table[c];
 }
 
 /* Convert to/from hex/sha1 representation */
diff --git a/sha1_file.c b/sha1_file.c
index a3637d7..e10fb4b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -33,7 +33,7 @@ const unsigned char null_sha1[20];
 
 static unsigned int sha1_file_open_flag = O_NOATIME;
 
-signed char hexval_table[256] = {
+const signed char hexval_table[256] = {
 	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 00-07 */
 	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 08-0f */
 	 -1, -1, -1, -1, -1, -1, -1, -1,		/* 10-17 */
@@ -72,11 +72,12 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
 {
 	int i;
 	for (i = 0; i < 20; i++) {
-		unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
+		unsigned char c0 = *hex++;
+		unsigned char c1 = *hex++;
+		unsigned int val = (hexval_table[c0] << 4) | hexval_table[c1];
 		if (val & ~0xff)
 			return -1;
 		*sha1++ = val;
-		hex += 2;
 	}
 	return 0;
 }

             reply	other threads:[~2007-05-30 17:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-30 17:09 Nicolas Pitre [this message]
2007-05-30 17:32 ` [PATCH] fix signed range problems with hex conversions Linus Torvalds
2007-05-30 17:42   ` Linus Torvalds
2007-05-30 18:20   ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.0.99.0705301213450.11491@xanadu.home \
    --to=nico@cam.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).