summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 071c41a)
raw | patch | inline | side by side (parent: 071c41a)
author | Ingo Molnar <mingo@elte.hu> | |
Thu, 14 Apr 2005 11:18:19 +0000 (13:18 +0200) | ||
committer | Petr Baudis <xpasky@machine.sinus.cz> | |
Wed, 11 May 2005 20:43:24 +0000 (22:43 +0200) |
this patch fixes a 1-byte overflow in update-cache.c (probably not
exploitable). A specially crafted db object might trigger this overflow.
the bug is that normally the 'type' field is parsed by read_sha1_file(),
via:
if (sscanf(buffer, "%10s %lu", type, size) != 2)
i.e. 0-10 long strings, which take 1-11 bytes of space. Normally the
type strings are stored in char [20] arrays, but in update-cache.c that
is char [10], so a 1 byte overflow might occur.
This should not happen with a 'friendly' DB, as the longest type string
("commit") is 7 bytes long. The fix is to use the customary char [20].
(someone might want to clean those open-coded constants up with a
TYPE_LEN define, they do tend to cause problems like this. I'm not
against open-coded constants (they make code much more readable), but
for fields that get filled in from possibly hostile objects this is
playing with fire.)
hey, this might be the first true security fix for GIT? ;-)
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Petr Baudis <pasky@ucw.cz>
exploitable). A specially crafted db object might trigger this overflow.
the bug is that normally the 'type' field is parsed by read_sha1_file(),
via:
if (sscanf(buffer, "%10s %lu", type, size) != 2)
i.e. 0-10 long strings, which take 1-11 bytes of space. Normally the
type strings are stored in char [20] arrays, but in update-cache.c that
is char [10], so a 1 byte overflow might occur.
This should not happen with a 'friendly' DB, as the longest type string
("commit") is 7 bytes long. The fix is to use the customary char [20].
(someone might want to clean those open-coded constants up with a
TYPE_LEN define, they do tend to cause problems like this. I'm not
against open-coded constants (they make code much more readable), but
for fields that get filled in from possibly hostile objects this is
playing with fire.)
hey, this might be the first true security fix for GIT? ;-)
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Petr Baudis <pasky@ucw.cz>
update-cache.c | patch | blob | history |
diff --git a/update-cache.c b/update-cache.c
index d7caed64f3384bac8f7a95992ce02c39190db4af..3b44fe14efd1398bdca6e7dd12a20507b4b5d60d 100644 (file)
--- a/update-cache.c
+++ b/update-cache.c
if (fd >= 0) {
void *buffer;
unsigned long size;
- char type[10];
+ char type[20];
buffer = read_sha1_file(ce->sha1, type, &size);
if (buffer) {