openbsd-ports/x11/windowmaker/patches/patch-WINGs_findfile_c
wilfried e1c9b28667 During expansion of path, the resulting path can overflow the
supplied area of PATH_MAX+2 (buffer as well as buffer2). A tampered
environment variable can be used to modify program flow.

Way too many functions handle a return value of NULL for wexpandpath
improperly, resulting in segfaults (and maybe other problems). To
prove the existance of these issues:

The improper parsing of environment variables can lead to expansion
of path names that were not intended to be expanded.

patch from Tobias Stoeckmann
2007-04-25 11:31:53 +00:00

128 lines
3.8 KiB
Plaintext

$OpenBSD: patch-WINGs_findfile_c,v 1.1 2007/04/25 11:31:53 wilfried Exp $
--- WINGs/findfile.c.orig Tue Oct 12 20:30:07 2004
+++ WINGs/findfile.c Wed Mar 14 21:16:22 2007
@@ -23,6 +23,7 @@
#include "WUtil.h"
+#include <errno.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
@@ -79,6 +80,7 @@ getuserhomedir(char *username)
char*
wexpandpath(char *path)
{
+ char *origpath = path;
char buffer2[PATH_MAX+2];
char buffer[PATH_MAX+2];
int i;
@@ -91,25 +93,29 @@ wexpandpath(char *path)
path++;
if (*path=='/' || *path==0) {
home = wgethomedir();
+ if (strlen(home) > PATH_MAX)
+ goto error;
strcat(buffer, home);
} else {
int j;
j = 0;
while (*path!=0 && *path!='/') {
+ if (j > PATH_MAX)
+ goto error;
buffer2[j++] = *path;
buffer2[j] = 0;
path++;
}
home = getuserhomedir(buffer2);
- if (!home)
- return NULL;
+ if (!home || strlen(home) > PATH_MAX)
+ goto error;
strcat(buffer, home);
}
}
i = strlen(buffer);
- while (*path!=0) {
+ while (*path!=0 && i <= PATH_MAX) {
char *tmp;
if (*path=='$') {
@@ -119,35 +125,50 @@ wexpandpath(char *path)
if (*path=='(') {
path++;
while (*path!=0 && *path!=')') {
+ if (j > PATH_MAX)
+ goto error;
buffer2[j++] = *(path++);
buffer2[j] = 0;
}
- if (*path==')')
+ if (*path==')') {
path++;
- tmp = getenv(buffer2);
+ tmp = getenv(buffer2);
+ } else {
+ tmp = NULL;
+ }
if (!tmp) {
+ if ((i += strlen(buffer2)+2) > PATH_MAX)
+ goto error;
buffer[i] = 0;
strcat(buffer, "$(");
strcat(buffer, buffer2);
- strcat(buffer, ")");
- i += strlen(buffer2)+3;
+ if (*(path-1)==')') {
+ if (++i > PATH_MAX)
+ goto error;
+ strcat(buffer, ")");
+ }
} else {
+ if ((i += strlen(tmp)) > PATH_MAX)
+ goto error;
strcat(buffer, tmp);
- i += strlen(tmp);
}
} else {
while (*path!=0 && *path!='/') {
+ if (j > PATH_MAX)
+ goto error;
buffer2[j++] = *(path++);
buffer2[j] = 0;
}
tmp = getenv(buffer2);
if (!tmp) {
+ if ((i += strlen(buffer2)+1) > PATH_MAX)
+ goto error;
strcat(buffer, "$");
strcat(buffer, buffer2);
- i += strlen(buffer2)+1;
} else {
+ if ((i += strlen(tmp)) > PATH_MAX)
+ goto error;
strcat(buffer, tmp);
- i += strlen(tmp);
}
}
} else {
@@ -156,7 +177,16 @@ wexpandpath(char *path)
}
}
+ if (*path!=0)
+ goto error;
+
return wstrdup(buffer);
+
+error:
+ errno = ENAMETOOLONG;
+ wsyserror(_("could not expand %s"), origpath);
+ /* FIXME: too many functions handle a return value of NULL incorrectly */
+ exit(1);
}