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>

2 files changed, 35 insertions, 11 deletions

diff --git a/modules/im/ximcp/Makefile.am b/modules/im/ximcp/Makefile.am
index 16a6ca87..8aae839c 100644
--- a/modules/im/ximcp/Makefile.am
+++ b/modules/im/ximcp/Makefile.am
@@ -6,6 +6,7 @@ AM_CPPFLAGS= \
 	-I$(top_srcdir)/src/xcms \
 	-I$(top_srcdir)/src/xkb \
 	-I$(top_srcdir)/src/xlibi18n \
+	-I$(top_srcdir)/src \
 	-D_BSD_SOURCE -DXIM_t -DTRANS_CLIENT
 
 AM_CFLAGS= \
diff --git a/modules/im/ximcp/imLcPrs.c b/modules/im/ximcp/imLcPrs.c
index bcf45791..f3627a0d 100644
--- a/modules/im/ximcp/imLcPrs.c
+++ b/modules/im/ximcp/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
 
@@ -307,9 +308,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 == '%') {
@@ -319,30 +320,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 == '%') {
@@ -374,6 +396,7 @@ TransFileName(Xim im, char *name)
       }
    }
    *j = '\0';
+end:
    Xfree(lcCompose);
    return ret;
 }