integer overflows in TransFileName() [CVE-2013-1981 9/13]

When trying to process file paths the tokens %H, %L, & %S are expanded
to $HOME, the standard compose file path & the xlocaledir path.
If enough of these tokens are repeated and values like $HOME are set to
very large values, the calculation of the total string size required to
hold the expanded path can overflow, resulting in allocating a smaller
string than the amount of data we'll write to it.

Simply restrict all of these values, and the total path size to PATH_MAX,
because really, that's all you should need for a filename path.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Backported-to-NX-by: Ulrich Sibiller <uli42@gmx.de>

1 files changed, 34 insertions, 11 deletions

diff --git a/nx-X11/lib/X11/imLcPrs.c b/nx-X11/lib/X11/imLcPrs.c
index 5b9a2844e..f02e93bbb 100644
--- a/nx-X11/lib/X11/imLcPrs.c
+++ b/nx-X11/lib/X11/imLcPrs.c
@@ -42,6 +42,7 @@ OR PERFORMANCE OF THIS SOFTWARE.
 #include <sys/stat.h>
 #include <stdio.h>
 #include <limits.h>
+#include "pathmax.h"
 
 #define XLC_BUFSIZE 256
 
@@ -305,9 +306,9 @@ static char*
 TransFileName(Xim im, char *name)
 {
    char *home = NULL, *lcCompose = NULL;
-   char dir[XLC_BUFSIZE];
-   char *i = name, *ret, *j;
-   int l = 0;
+   char dir[XLC_BUFSIZE] = "";
+   char *i = name, *ret = NULL, *j;
+   size_t l = 0;
 
    while (*i) {
       if (*i == '%') {
@@ -317,30 +318,51 @@ TransFileName(Xim im, char *name)
                  l++;
    	         break;
    	      case 'H':
-   	         home = getenv("HOME");
-   	         if (home)
-                     l += strlen(home);
+                 if (home == NULL)
+                     home = getenv("HOME");
+                 if (home) {
+                     size_t Hsize = strlen(home);
+                     if (Hsize > PATH_MAX)
+                         /* your home directory length is ridiculous */
+                         goto end;
+                     l += Hsize;
+                 }
    	         break;
    	      case 'L':
                  if (lcCompose == NULL)
                      lcCompose = _XlcFileName(im->core.lcd, COMPOSE_FILE);
-                 if (lcCompose)
-                     l += strlen(lcCompose);
+                 if (lcCompose) {
+                     size_t Lsize = strlen(lcCompose);
+                     if (Lsize > PATH_MAX)
+                         /* your compose pathname length is ridiculous */
+                         goto end;
+                     l += Lsize;
+                 }
    	         break;
    	      case 'S':
-                 xlocaledir(dir, XLC_BUFSIZE);
-                 l += strlen(dir);
+                 if (dir[0] == '\0')
+                     xlocaledir(dir, XLC_BUFSIZE);
+                 if (dir[0]) {
+                     size_t Ssize = strlen(dir);
+                     if (Ssize > PATH_MAX)
+                         /* your locale directory path length is ridiculous */
+                         goto end;
+                     l += Ssize;
+                 }
    	         break;
    	  }
       } else {
       	  l++;
       }
       i++;
+      if (l > PATH_MAX)
+          /* your expanded path length is ridiculous */
+          goto end;
    }
 
    j = ret = Xmalloc(l+1);
    if (ret == NULL)
-      return ret;
+      goto end;
    i = name;
    while (*i) {
       if (*i == '%') {
@@ -372,6 +394,7 @@ TransFileName(Xim im, char *name)
       }
    }
    *j = '\0';
+end:
    Xfree(lcCompose);
    return ret;
 }