[8430] in bugtraq
xlock mishandles malformed .signature/.plan
daemon@ATHENA.MIT.EDU (Aaron Campbell)
Thu Nov 5 17:57:31 1998
Date: Thu, 5 Nov 1998 02:41:41 -0400
Reply-To: aaron@UG.CS.DAL.CA
From: Aaron Campbell <aaron@UG.CS.DAL.CA>
To: BUGTRAQ@NETSPACE.ORG
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. At any rate, a bug that should definitely be
fixed, especially since xlock is normally set-user-ID root.
The maintainer of xlockmore has been notified. Below is a patch from Todd
Miller that has already been applied to OpenBSD -current. Please save this
then edit instead of copy/paste, a couple of lines are longer than 80
columns.
And of course, if it doesn't apply, try the -l flag to patch(1) in case the
whitespace messed up somewhere along the way before you e-mail me to
complain it doesn't work.
. _ _ _ _ . . _ _ . . _ _ _ . .
: |-||-||<|_||\| |_|-||\/||-'|->|_-|_|_ DalTech, Halifax, NS, Canada
`---------------------------------------- [http://www.biodome.org/~fx] -
--- xlock.c.orig Wed Nov 4 20:33:47 1998
+++ xlock.c Wed Nov 4 20:34:28 1998
@@ -2524,7 +2524,7 @@
char buf[121];
char *home = getenv("HOME");
char *buffer;
- int i, j, cr;
+ int i, j, len;
if (!home)
home = "";
@@ -2587,13 +2587,12 @@
}
if (planf != NULL) {
for (i = 0; i < TEXTLINES; i++) {
- if (fgets(buf, 120, planf)) {
- cr = strlen(buf) - 1;
- if (buf[cr] == '\n') {
- buf[cr] = '\0';
+ if (fgets(buf, 120, planf) && (len = strlen(buf)) > 0) {
+ if (buf[len - 1] == '\n') {
+ buf[--len] = '\0';
}
/* this expands tabs to 8 spaces */
- for (j = 0; j < cr; j++) {
+ for (j = 0; j < len; j++) {
if (buf[j] == '\t') {
int k, tab = 8 - (j % 8);
@@ -2603,12 +2602,11 @@
for (k = j; k < j + tab; k++) {
buf[k] = ' ';
}
- cr += tab;
- if (cr > 120)
- cr = 120;
+ len += tab;
+ if (len > 120)
+ len = 120;
}
}
- buf[cr] = '\0';
plantext[i] = (char *) malloc(strlen(buf) + 1);
(void) strcpy(plantext[i], buf);
