Re: [patch] git: fix 1-byte overflow in show-files.c

2005-04-18 Thread Ingo Molnar

* Petr Baudis <[EMAIL PROTECTED]> wrote:

> > will attempt to append a "/" string to the directory name - resulting in 
> > a 1-byte overflow (a zero byte is written to offset 4097, which is 
> > outside the array).
> 
> The name ends precisely at offset 4095 with its NUL character:
> 
>  {PATH_MAX}
>  Maximum number of bytes in a pathname, including the terminating
> null character.
> [ http://www.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html ]
> 
> So, if I'm not mistaken, '/' will be written at offset 4095 instead of 
> the NUL and the NUL will be written at 4096. Everything's fine, right?

yeah, you are right - ignore this patch.

Ingo
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] git: fix 1-byte overflow in show-files.c

2005-04-17 Thread Petr Baudis
Dear diary, on Thu, Apr 14, 2005 at 02:53:54PM CEST, I got a letter
where Ingo Molnar <[EMAIL PROTECTED]> told me that...
> 
> this patch fixes a 1-byte overflow in show-files.c (looks narrow is is 
> probably not exploitable). A specially crafted db object (tree) might 
> trigger this overflow.
> 
> 'fullname' is an array of 4096+1 bytes, and we do readdir(), which 
> produces entries that have strings with a length of 0-255 bytes. With a 
> long enough 'base', it's possible to construct a tree with a name in it 
> that has directory whose name ends precisely at offset 4095. At that 
> point this code:
> 
> case DT_DIR:
> memcpy(fullname + baselen + len, "/", 2);
> 
> will attempt to append a "/" string to the directory name - resulting in 
> a 1-byte overflow (a zero byte is written to offset 4097, which is 
> outside the array).

The name ends precisely at offset 4095 with its NUL character:

 {PATH_MAX}
 Maximum number of bytes in a pathname, including the terminating
null character.
[ http://www.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html ]

So, if I'm not mistaken, '/' will be written at offset 4095 instead of
the NUL and the NUL will be written at 4096. Everything's fine, right?

-- 
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch] git: fix 1-byte overflow in show-files.c

2005-04-14 Thread Ingo Molnar

this patch fixes a 1-byte overflow in show-files.c (looks narrow is is 
probably not exploitable). A specially crafted db object (tree) might 
trigger this overflow.

'fullname' is an array of 4096+1 bytes, and we do readdir(), which 
produces entries that have strings with a length of 0-255 bytes. With a 
long enough 'base', it's possible to construct a tree with a name in it 
that has directory whose name ends precisely at offset 4095. At that 
point this code:

case DT_DIR:
memcpy(fullname + baselen + len, "/", 2);

will attempt to append a "/" string to the directory name - resulting in 
a 1-byte overflow (a zero byte is written to offset 4097, which is 
outside the array).

Ingo

Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>

--- show-files.c.orig
+++ show-files.c
@@ -49,7 +49,7 @@ static void read_directory(const char *p
 
if (dir) {
struct dirent *de;
-   char fullname[MAXPATHLEN + 1];
+   char fullname[MAXPATHLEN + 2]; // +1 byte for trailing slash
memcpy(fullname, base, baselen);
 
while ((de = readdir(dir)) != NULL) {

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html