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;
}
next 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).