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