Re: Buffer overflow in name_to_tcb

Lasse Kliemann <[email protected]> wrote:

> > > A look in star.h tells that t_name is only NAMSIZ long, which is 100.
>
> > It seems that you are a victim of a C-compiler bug.
> > =

> > There is nothing real that would allow you tp call that a buffer overfl=
ow as =

> > it is 100% legal in the tar standard to put up to 100 chars into the t_=
name =

> > array.
>
> String length is 100, so strcpy will try to put 101 chars into =

> the t_name array, since it appends a null character.

There still is a difference: in star, the usage of the data structures is =

defined by a standard and star does never even touch any byte outside the d=
ata =

structures. =


> > In theory I could use strncpy but this would result in a noticable slow=
down or =

> > at least performance penalty.
> > =

> > This is why star carefully fills up the tar header in the right order. =

>
> OK, but the protection mechanism has no way of knowing this.

This would not be discussed at the expence of GCC if GCC would not create =

incorrect warnings since a long time just because the parser in gcc ist lou=
sy.

GCC gives tons of false warnings about supposed unintialized data.

If you belive these incorrect warnings and change your code, you will cause=
 the =

IRIX compiler to warn about "useless code" and emit things like "warning =

multiple intialization without use of the variable".

If I like to know whether there is a non-obvious missing intialization, I a=
sk =

"lint" from the Sun C-compiler because it gives less false warnings.

> Overstepping of array bounds often is an indication that =

> something is not working out as intended by the programmer. While =

> this is not the case here, the warning is still justified.

If you look at code from experienced programmers, there is a bigger chance =
to =

get overstepping from library functions. A few years ago, I detected a bug =
in
memcmp() in the Solaris kernel that caused a kernel panic because it steppe=
d =

into an umnapped kernel page.


> > A major problem with recent GCCs is that they under some circumstances =
ignore =

> > the C standard. The code fragment:
> > =

> > 	(void)write(STDERR_FILENO, "test\n", 5);
> > =

> > is not expected to create a warning. It however causes a non-avoidable =
warning =

> > on revent Linux versions.
>
> I get:
>
> $ gcc test.c -Wall -fstack-protector -D_FORTIFY_SOURCE=3D2 -O2
> test.c:2:6: warning: return type of ???main??? is not ???int???
> test.c: In function ???main???:
> test.c:3:1: warning: ignoring return value of ???write???, declared with =
attribute warn_unused_result

So you may now understzand why the reputation for GCC related to warnings i=
s =

bad. You just verified a defintive compiler bug.

You here also see the main problem in gcc:

it ignores even standard methods to give hints. It ignores (void) casts that
in my example tell the compiler: I intentionally ignore the return code at =
this =

location. And gcc in addition ignores the standard method that is in use si=
nce =

30 years: standardized comments like:

/* LINTED */
/* EMPTY */
/* FALLTHRU */
/* ARGSUSED */
....



> > Did you have a specific reason to use special compiler options?
> > Array bound checking causes a slow down of the code.
>
> It is a security measure. Roughly 95% of my system is compiled =

> with stack protector and the fortify source feature. For a few =

> exceptions, it has to be switched off, e.g., Glibc needs some =

> special tuning and a few other packages. And for some packages, =

> I have not figured out yet how to pass CFLAGS to the build =

> system. But the vast majority of my packages is compiled with the =

> protection features turned on.

See the file COMPILE for instructions.


> > There is a trick to make over-picky compilers quiet......
> > =

> > change strcpy(ptb->dbuf.t_name, name); to:
> > =

> > 	strcpy(ptb->ndbuf.t_name, name);
> > =

> > and try again.....
>
> This works.
>
> Is it intended as a permanent fix?

This is seen as a workaround as an intentionally wrong data structure varia=
nt =

cannot be seen as a fix.

> > BTW: would you be interested to write automated test cases?
> > =

> > I recently wrote a test case that I currently use for SCCS, but I would=
 like to =

> > write a test suite for star too.
>
> A test suite for star would be good. I'm not sure whether I can =

> give valuable contributions since I do not know much of the =

> internals, so I do not know all the corner cases which need to be =

> tested.

Well, I wrote test code for star during the past 30 years and I would call =
star =

the most mature tar implementation, but I did not keep the test code as it =
was =

never written in a way that allowed to be run automatically.

We need a larger test suite and writing such a test suite may take a long t=
ime..

J=F6rg

-- =

 EMail:[email protected] (home) J=F6rg Schilling D-13353 Be=
rlin
       [email protected]                (uni)  =

       [email protected] (work) Blog: http://schily.blogs=
pot.com/
 URL:  http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily