[8449] in bugtraq
Re: xlock mishandles malformed .signature/.plan
daemon@ATHENA.MIT.EDU (Jochen Thomas Bauer)
Fri Nov 6 21:31:56 1998
Date: Fri, 6 Nov 1998 17:44:20 +0100
Reply-To: Jochen Thomas Bauer <jtb@THEO2.PHYSIK.UNI-STUTTGART.DE>
From: Jochen Thomas Bauer <jtb@THEO2.PHYSIK.UNI-STUTTGART.DE>
To: BUGTRAQ@NETSPACE.ORG
Aaron Campbell wrote:
>I found a bug last night in xlock that I felt should be known.
>xlock iteratively searches for .xlocktext, .plan, and then a .signature
>file in the invoker's home directory, and upon finding one, opens it for
>reading. The problem is here, in xlock/xlock.c, under the read_plan()
>function:
>
>static void
>read_plan()
>{
> FILE *planf = NULL;
> char buf[121];
> char *home = getenv("HOME");
> char *buffer;
> int i, j, cr;
>[...]
> planf = my_fopen(buffer, "r");
> }
> if (planf != NULL) {
> for (i = 0; i < TEXTLINES; i++) {
> if (fgets(buf, 120, planf)) {
> cr = strlen(buf) - 1;
>[...]
> buf[cr] = '\0';
>[...]
>
>If we generate a poisonous .signature file, for example, containing at
>least one line that begins with a NULL byte, fgets() will evaluate to true
>but strlen(buf) will return 0 and cr will point outside of buf, obviously
>bad.
>It's hard to tell how serious this is, but I'm sure it could be harmful in
>some situations/environments.
I think it is not too serious, as only the following thing will happen:
(well, at least on an Intel x86)
The local variable "char *home", that will be filled with a pointer to a string
containing the path to the user's home directory, is declared right after the
local varibale "char buf[121]", so this pointer will be located right above the
space left for "char buf[121]" on the stack (remember, the stack grows to
lower addresses on an Intel x86). This means that if fgets returns NULL and
therefore c==-1, buf[cr] = '\0' will overwrite the most significant byte
of the pointer "char *home" with NULL (the least significant byte if you are on a
big endian machine). However, if you take a closer look at the
code you will see that
1.)
cr = strlen(buf) - 1;
if (buf[cr] == '\n') {
buf[cr] = '\0';
}
So buf[-1] must have a value of buf[-1]=10 in order to get overwritten by NULL.
2.)
The pointer "char *home" has at this point already been used to construct the
full path to the .plan, .signature or .xlocktext file, and the result has already
been written to a buffer pointed to by "char *buffer".
So, IMHO, there is no security hole created by this bug.
Jochen Bauer
Institute for Theoretical Physics
University of Stuttgart, Germany
