from:"Peter Jones"

[Python-Dev] unintentional and unsafe use of realpath()

2005-09-10 Thread Peter Jones

Hi,

In Python 2.4.1, Python/sysmodule.c includes a function PySys_SetArgv().
One of the things it does is attempt to resolve symbolic links into
absolute paths.  Currently, it uses readlink() if configure found that
your system supports it, and then it tries to do the same thing again
using realpath() if you system supports that.

This seems wrong; there's really no reason to do both.  So here's a
patch to move the realpath() usage into a #else following the
HAVE_READLINK test:

--- Python-2.4.1/Python/sysmodule.c.readlink2005-09-10 14:05:26.0 
-0400
+++ Python-2.4.1/Python/sysmodule.c 2005-09-10 14:06:00.0 -0400
@@ -1211,7 +1211,7 @@
}
}
}
-#endif /* HAVE_READLINK */
+#else /* HAVE_READLINK */
 #if SEP == '\\' /* Special case for MS filename syntax */
if (argc > 0 && argv0 != NULL) {
char *q;
@@ -1244,6 +1244,7 @@
 #endif
p = strrchr(argv0, SEP);
}
+#endif /* HAVE_READLINK */
if (p != NULL) {
 #ifndef RISCOS
n = p + 1 - argv0;


Another problem (which I have not fixed) is that when realpath() is
used, in some cases MAXPATHLEN is smaller than the system's
PATH_MAX/pathconf(path, _PC_PATH_MAX).  When that happens, the
realpath() usage is a potential buffer overflow, which can be
selectively aggravated using a carefully constructed symbolic link.
This is the case currently on Fedora Core (Linux) at least, and possibly
other OSes.

Recent versions of gcc include a feature to check for this kind of bug
at runtime, and if you build python with gcc+glibc and the gcc command
line arguments "-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector"
it will give you a failure that looks something like:

case $MAKEFLAGS in \
*-s*) LD_LIBRARY_PATH=/home/pjones/build/BUILD/Python-2.4.1: CC='gcc
-pthread' LDSHARED='gcc -pthread -shared' OPT='-O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=pentium4
-fasynchronous-unwind-tables -D_GNU_SOURCE -fPIC -I/usr/kerberos/include
' ./python -E ./setup.py -q build;; \
*) LD_LIBRARY_PATH=/home/pjones/build/BUILD/Python-2.4.1: CC='gcc
-pthread' LDSHARED='gcc -pthread -shared' OPT='-O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=pentium4
-fasynchronous-unwind-tables -D_GNU_SOURCE -fPIC -I/usr/kerberos/include
' ./python -E ./setup.py build;; \
esac
*** buffer overflow detected ***: ./python terminated
=== Backtrace: =
/lib/libc.so.6(__chk_fail+0x41)[0x590495]
/lib/libc.so.6(__ptsname_r_chk+0x0)[0x590ac0]
/home/pjones/build/BUILD/Python-2.4.1/libpython2.4.so.1.0(PySys_SetArgv
+0x1a1)[0x228cf5]
/home/pjones/build/BUILD/Python-2.4.1/libpython2.4.so.1.0(Py_Main
+0x671)[0x22bedd]
./python(main+0x2a)[0x804859a]
/lib/libc.so.6(__libc_start_main+0xdf)[0x4c74ff]
./python[0x80484ed]
=== Memory map: 
00185000-0026e000 r-xp  03:03
278531 /home/pjones/build/BUILD/Python-2.4.1/libpython2.4.so.1.0
0026e000-00295000 rwxp 000e8000 03:03
278531 /home/pjones/build/BUILD/Python-2.4.1/libpython2.4.so.1.0
00295000-00298000 rwxp 00295000 00:00 0
00495000-004ae000 r-xp  03:03 1966150/lib/ld-2.3.90.so
004ae000-004af000 r-xp 00018000 03:03 1966150/lib/ld-2.3.90.so
004af000-004b rwxp 00019000 03:03 1966150/lib/ld-2.3.90.so
004b2000-005d7000 r-xp  03:03 1966261/lib/libc-2.3.90.so
005d7000-005d9000 r-xp 00124000 03:03 1966261/lib/libc-2.3.90.so
005d9000-005db000 rwxp 00126000 03:03 1966261/lib/libc-2.3.90.so
005db000-005dd000 rwxp 005db000 00:00 0
005df000-00602000 r-xp  03:03 1966272/lib/libm-2.3.90.so
00602000-00603000 r-xp 00022000 03:03 1966272/lib/libm-2.3.90.so
00603000-00604000 rwxp 00023000 03:03 1966272/lib/libm-2.3.90.so
00606000-00608000 r-xp  03:03 1966270/lib/libdl-2.3.90.so
00608000-00609000 r-xp 1000 03:03 1966270/lib/libdl-2.3.90.so
00609000-0060a000 rwxp 2000 03:03 1966270/lib/libdl-2.3.90.so
006f7000-00705000 r-xp  03:03
1966283/lib/libpthread-2.3.90.so
00705000-00706000 r-xp d000 03:03
1966283/lib/libpthread-2.3.90.so
00706000-00707000 rwxp e000 03:03
1966283/lib/libpthread-2.3.90.so
00707000-00709000 rwxp 00707000 00:00 0
0073f000-00743000 r-xp  03:03
1442230/home/pjones/build/BUILD/Python-2.4.1/Modules/stropmodule.so
00743000-00745000 rwxp 4000 03:03
1442230/home/pjones/build/BUILD/Python-2.4.1/Modules/stropmodule.so
00a22000-00a2b000 r-xp  03:03
1966127/lib/libgcc_s-4.0.1-20050906.so.1
00a2b000-00a2c000 rwxp 9000 03:03
1966127/lib/libgcc_s-4.0.1-20050906.so.1
00df7000-00df9000 r-xp  03:03 1968751/lib/libutil-2.3.90.so
00df9000-00dfa000 r-xp 1000 03:03 1968751/lib/libutil-2.3.90.so
00dfa000-00dfb000 rwxp 2000 03

Re: [Python-Dev] unintentional and unsafe use of realpath()

2005-09-12 Thread Peter Jones

On Mon, 2005-09-12 at 17:23 +1200, Greg Ewing wrote:
> Peter Jones wrote:
> 
> > Another problem (which I have not fixed) is that when realpath() is
> > used, in some cases MAXPATHLEN is smaller than the system's
> > PATH_MAX/pathconf(path, _PC_PATH_MAX).
> 
> The linux man page for realpath() has this at the bottom:
> 
> BUGS
> Never  use this function. It is broken by design since it is 
> impossible
> to determine a suitable size for the output buffer.  According to 
> POSIX
> a  buffer of size PATH_MAX suffices, but PATH_MAX need not be a 
> defined
> constant, and may have to be obtained  using  pathconf().   And  
> asking
> pathconf() does not really help, since on the one hand POSIX warns 
> that
> the result of pathconf() may be huge and unsuitable for mallocing  
> mem-
> ory.  And  on  the  other hand pathconf() may return -1 to signify 
> that
> PATH_MAX is not bounded.
> 
> So maybe it shouldn't be using realpath() at all?

Well, the intent is clearly that OSes that have a better option, we
shouldn't be using it.

But that wasn't really my second point.  When use of realpath is
unavoidable, it is vitally important that MAXPATHLEN is set to the same
value that realpath() got when it was built.  If it's any smaller, it's
a straightforward overflow, with something like $SYSTEM_MAXPATHLEN -
$PYTHON_MAXPATHLEN of space to write nasty bits into.  At least on one
box I looked at, system's was 4096 and python wound up doing the
fallback of 256 (I'm not entirely sure why) so that's 3768 bytes of
stack potentially overwritten.

In this case, it's not realistically exploitable, because it means a
user has to trick root into running "python foo" where foo is a symlink
that's built terrifyingly weirdly.  So since the user is supplying the
symlink, there are much more trivial attacks.  But I haven't checked all
the uses of realpath in python, some of them could be dangerous.

-- 
  Peter

___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] unintentional and unsafe use of realpath()

2005-09-14 Thread Peter Jones

[re-adding Python-Dev]

On Wed, 2005-09-14 at 02:00 +0200, Henrik Levkowetz wrote:
> Hi Peter,
> 
> on 2005-09-10 21:29 Peter Jones said the following:
> > Hi,
> > 
> > In Python 2.4.1, Python/sysmodule.c includes a function PySys_SetArgv().
> > One of the things it does is attempt to resolve symbolic links into
> > absolute paths.  Currently, it uses readlink() if configure found that
> > your system supports it, and then it tries to do the same thing again
> > using realpath() if you system supports that.
> > 
> > This seems wrong; there's really no reason to do both.  So here's a
> > patch to move the realpath() usage into a #else following the
> > HAVE_READLINK test:
> > 
> 
> If a path component above the basename is a link, it will not be
> resolved by only doing readlink() on the basename, while realpath() will
> resolve it.  So it seems to me that your proposed patch will accomplish
> less than the current code for systems which have realpath().

What the current code is doing is a buffer overrun.  Nevertheless,
you're right that it would behave differently than the current code if
that code worked as intended.

Here's a different patch; it uses canonicalize_file_name instead, and if
you don't have that it just uses the same codepath that's already in the
tree.  canonicalize_file_name is identical to realpath() when using the
glibc extension of allocating a buffer when you pass in a NULL target,
as in:

fullpath = realpath(path, NULL);

I chose to use canonicalize_file_name simply because the autoconf test
for it is easier than testing to see if the extension works.

Here's the new patch:

--- Python-2.4.1/pyconfig.h.in.canonicalize 2005-09-14 11:47:04.0 
-0400
+++ Python-2.4.1/pyconfig.h.in  2005-09-14 11:47:02.0 -0400
@@ -58,6 +58,9 @@
 /* Define if pthread_sigmask() does not work on your system. */
 #undef HAVE_BROKEN_PTHREAD_SIGMASK
 
+/* Define to 1 if you have the `canonicalize_file_name' function. */
+#undef HAVE_CANONICALIZE_FILE_NAME
+
 /* Define to 1 if you have the `chown' function. */
 #undef HAVE_CHOWN
 
--- Python-2.4.1/Python/sysmodule.c.canonicalize2005-09-14 
11:53:30.0 -0400
+++ Python-2.4.1/Python/sysmodule.c 2005-09-14 11:52:04.0 -0400
@@ -1184,6 +1184,9 @@
char *p = NULL;
int n = 0;
PyObject *a;
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+argv0 = canonicalize_file_name(argv0);
+#else /* ! HAVE_CANONICALIZE_FILE_NAME */
 #ifdef HAVE_READLINK
char link[MAXPATHLEN+1];
char argv0copy[2*MAXPATHLEN+1];
@@ -1256,6 +1259,7 @@
 #endif /* Unix */
}
 #endif /* All others */
+#endif /* ! HAVE_CANONICALIZE_FILE_NAME */
a = PyString_FromStringAndSize(argv0, n);
if (a == NULL)
Py_FatalError("no mem for sys.path insertion");
--- Python-2.4.1/configure.in.canonicalize  2005-09-14 11:46:00.0 
-0400
+++ Python-2.4.1/configure.in   2005-09-14 11:47:22.0 -0400
@@ -2096,8 +2096,8 @@
 AC_MSG_RESULT(MACHDEP_OBJS)
 
 # checks for library functions
-AC_CHECK_FUNCS(alarm bind_textdomain_codeset chown clock confstr ctermid \
- execv fork fpathconf ftime ftruncate \
+AC_CHECK_FUNCS(alarm bind_textdomain_codeset canonicalize_file_name chown \
+ clock confstr ctermid execv fork fpathconf ftime ftruncate \
  gai_strerror getgroups getlogin getloadavg getpeername getpgid getpid \
  getpriority getpwent getsid getwd \
  kill killpg lchown lstat mkfifo mknod mktime \

-- 
  Peter

___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] unintentional and unsafe use of realpath()

2005-09-21 Thread Peter Jones

On Wed, 2005-09-14 at 15:25 -0400, Peter Jones wrote:
[ comments and a patch for sysmodule.c and some configure related files]

... and that patch has obvious problems as well.

Here's a corrected one:

--- Python-2.4.1/pyconfig.h.in.canonicalize 2005-09-14 11:47:04.0 
-0400
+++ Python-2.4.1/pyconfig.h.in  2005-09-14 11:47:02.0 -0400
@@ -58,6 +58,9 @@
 /* Define if pthread_sigmask() does not work on your system. */
 #undef HAVE_BROKEN_PTHREAD_SIGMASK
 
+/* Define to 1 if you have the `canonicalize_file_name' function. */
+#undef HAVE_CANONICALIZE_FILE_NAME
+
 /* Define to 1 if you have the `chown' function. */
 #undef HAVE_CHOWN
 
--- Python-2.4.1/Python/sysmodule.c.canonicalize2005-09-14 
11:53:30.0 -0400
+++ Python-2.4.1/Python/sysmodule.c 2005-09-14 11:52:04.0 -0400
@@ -1184,6 +1184,11 @@
char *p = NULL;
int n = 0;
PyObject *a;
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+   argv0 = canonicalize_file_name(argv0);
+   if (argv0 == NULL)
+   Py_FatalError("no mem for sys.argv");
+#else /* ! HAVE_CANONICALIZE_FILE_NAME */
 #ifdef HAVE_READLINK
char link[MAXPATHLEN+1];
char argv0copy[2*MAXPATHLEN+1];
@@ -1256,9 +1261,13 @@
 #endif /* Unix */
}
 #endif /* All others */
+#endif /* ! HAVE_CANONICALIZE_FILE_NAME */
a = PyString_FromStringAndSize(argv0, n);
if (a == NULL)
Py_FatalError("no mem for sys.path insertion");
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+   free(argv0);
+#endif /* HAVE_CANONICALIZE_FILE_NAME */
if (PyList_Insert(path, 0, a) < 0)
Py_FatalError("sys.path.insert(0) failed");
Py_DECREF(a);
--- Python-2.4.1/configure.in.canonicalize  2005-09-14 11:46:00.0 
-0400
+++ Python-2.4.1/configure.in   2005-09-14 11:47:22.0 -0400
@@ -2096,8 +2096,8 @@
 AC_MSG_RESULT(MACHDEP_OBJS)
 
 # checks for library functions
-AC_CHECK_FUNCS(alarm bind_textdomain_codeset chown clock confstr ctermid \
- execv fork fpathconf ftime ftruncate \
+AC_CHECK_FUNCS(alarm bind_textdomain_codeset canonicalize_file_name chown \
+ clock confstr ctermid execv fork fpathconf ftime ftruncate \
  gai_strerror getgroups getlogin getloadavg getpeername getpgid getpid \
  getpriority getpwent getsid getwd \
  kill killpg lchown lstat mkfifo mknod mktime \

-- 
  Peter

___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

[Python-Dev] unintentional and unsafe use of realpath()

Re: [Python-Dev] unintentional and unsafe use of realpath()

Re: [Python-Dev] unintentional and unsafe use of realpath()

Re: [Python-Dev] unintentional and unsafe use of realpath()

4 matches

Site Navigation

Mail list logo

Footer information