git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fix signed range problems with hex conversions
@ 2007-05-30 17:09 Nicolas Pitre
  2007-05-30 17:32 ` Linus Torvalds
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Pitre @ 2007-05-30 17:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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;
 }

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-05-30 18:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-30 17:09 [PATCH] fix signed range problems with hex conversions Nicolas Pitre
2007-05-30 17:32 ` Linus Torvalds
2007-05-30 17:42   ` Linus Torvalds
2007-05-30 18:20   ` Nicolas Pitre

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).