diff options
author | Alan Coopersmith <alan.coopersmith@oracle.com> | 2013-03-02 13:18:48 -0800 |
---|---|---|
committer | Alan Coopersmith <alan.coopersmith@oracle.com> | 2013-05-09 18:59:52 -0700 |
commit | 164bf4dfe839b1cc75cdeee378a243d04a8200e4 (patch) | |
tree | b0d2ff3876da474ee0c3d45e6dfef22d2f6cad8f | |
parent | 460e8a223b87d4fa0ea1e97823e998a770e0f2a2 (diff) |
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>
-rw-r--r-- | modules/im/ximcp/Makefile.am | 1 | ||||
-rw-r--r-- | modules/im/ximcp/imLcPrs.c | 45 |
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; } |