Bugtraq mailing list archives
Re: xlock mishandles malformed .signature/.plan
From: tschweik () FIDUCIA DE (tschweik () FIDUCIA DE)
Date: Mon, 9 Nov 1998 15:06:31 +0100
Jochen Bauer wrote:
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.
[snip] Stating ANSI-C that's wrong. The compiler is allowed to reorder local variables --- the scenario described above may happen like that, but only MAY. It depends on the compiler used to create the binaries and optimizations switched on for this compiler. Additionally some compilers don't really create a buffer the way you where expecting it: embedded in between the other variables. Some compilers include code doing malloc for the space needed at function entry. At function exit the space is freed again. Overhead is bigger this way, but stack space is preserved. Results are absolutely unpredictable if unintentionally overwriting a byte that way. -- Thomas Schweikle <tschweik () fiducia de>
Current thread:
- Re: xlock mishandles malformed .signature/.plan Jochen Thomas Bauer (Nov 06)
- Re: xlock mishandles malformed .signature/.plan Aaron Campbell (Nov 07)
- shadow problems. twiztah (Nov 08)
- tcpd -DPARANOID doesn't work, and never did D. J. Bernstein (Nov 08)
- Re: tcpd -DPARANOID doesn't work, and never did Warner Losh (Nov 09)
- Re: tcpd -DPARANOID doesn't work, and never did Peter Wemm (Nov 09)
- Re: tcpd -DPARANOID doesn't work, and never did Wietse Venema (Nov 10)
- Re: tcpd -DPARANOID doesn't work, and never did Warner Losh (Nov 09)
- Re: tcpd -DPARANOID doesn't work, and never did Wietse Venema (Nov 09)
- Re: tcpd -DPARANOID doesn't work, and never did Chip Christian (Nov 10)
- <Possible follow-ups>
- Re: xlock mishandles malformed .signature/.plan tschweik () FIDUCIA DE (Nov 09)
- Re: xlock mishandles malformed .signature/.plan tschweik () FIDUCIA DE (Nov 11)