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>

integer overflows in TransFileName() [CVE-2013-1981 9/13]
25172302 · Alan Coopersmith · Ulrich Sibiller · 8468165a · 25172302
Commit 25172302 authored Mar 02, 2013 by Alan Coopersmith Committed by Ulrich Sibiller Oct 12, 2016
Hide whitespace changes
Inline Side-by-side

Showing with 34 additions and 11 deletions

imLcPrs.c nx-X11/lib/X11/imLcPrs.c +34 -11

No files found.
--- 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;
 }