From 1db15a35432b9b954b5b14288f6a1a0f3a568646 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <tobias@openbsd.org>
Date: Sun, 18 Feb 2007 14:48:33 +0100
Subject: [PATCH] Fix multiple errors in findfile.c

Problems:

1.

  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.

Proof:
      [note: wmaker has been compiled with propolice]
      $ export A="[tested with 4096x A]"
      $ GNUSTEP_USER_ROOT="\$A\$A/\$A/\$A/" wmaker --for-real
      *** stack smashing detected ***: wmaker terminated
      Aborted

2.

  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:

Proof:
      $ GNUSTEP_USER_ROOT=~nouser wmaker --for-real
      wmaker error: could not get password entry for user nouser: Success
      Segmentation fault

Solution:

     hard exit with error message about what is going on.

3.

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

     (a) If a string like "$(var" is found, Window Maker tries to expand "var"
     	 (environment variable) although the syntax is wrong.

Proof:

      $ export PROOF=foo
      $ GNUSTEP_USER_ROOT=/\$\(PROOF wmaker --for-real
      wmaker warning: could not find user GNUstep directory (/foo/Defaults/WindowMaker).

      (b) If the variable out of a) cannot be resolved, a closing bracket will be
      	  added.

Proof:

      $ unset PROOF
      $ GNUSTEP_USER_ROOT=/\$\(PROOF wmaker --for-real
      ./wmaker warning: could not find user GNUstep directory ($(PROOF)/Defaults/WindowMaker).

Author: Tobias Stoeckmann
Retrieved-from: http://paldium.homeunix.org/tobias/wmaker/
Submitted-by: Gilbert Ashley <amigo@ibiblio.org>
---
 WINGs/findfile.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/WINGs/findfile.c b/WINGs/findfile.c
index c718b4b5..d5699e6f 100644
--- a/WINGs/findfile.c
+++ b/WINGs/findfile.c
@@ -22,6 +22,7 @@
 
 #include "WUtil.h"
 
+#include <errno.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
@@ -70,6 +71,7 @@ static char *getuserhomedir(char *username)
 
 char *wexpandpath(char *path)
 {
+	char *origpath = path;
 	char buffer2[PATH_MAX + 2];
 	char buffer[PATH_MAX + 2];
 	int i;
@@ -82,25 +84,29 @@ char *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 == '$') {
@@ -110,35 +116,50 @@ char *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 {
@@ -147,7 +168,16 @@ char *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);
 }
 
 /* return address of next char != tok or end of string whichever comes first */
-- 
2.11.4.GIT