[PATCH] fix signed range problems with hex conversions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
---

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;
 }
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux