| msg21941 - (view) |
Author: Kirby Kuehl (vacuum) |
Date: 2004-08-04 18:42 |
| First off, I realize that Python 1.5.2 is old, I am just reporting it to be thorough. I was doing a security audit of an application that used this old version of python and found the following bug in the joinpath function. #0 0xff133bf0 in strncpy () from /usr/lib/libc.so.1 (gdb) bt #0 0xff133bf0 in strncpy () from /usr/lib/libc.so.1 (gdb) bt #0 0xff133bf0 in strncpy () from /usr/lib/libc.so.1 #1 0x000304e0 in joinpath (buffer=0x83528 'A' <repeats 200 times>..., stuff=0x84140 'A' <repeats 200 times>...) at ./getpath.c:255 #2 0x00030604 in search_for_prefix (argv0_path=0xffbff530 "/opt/OPSW/bin", home=0xff3a0840 'A' <repeats 200 times>...) at ./getpath.c:300 #3 0x00030a48 in calculate_path () at ./getpath.c:481 #4 0x00030e6c in Py_GetProgramFullPath () at ./getpath.c:634 #5 0x0002dcac in _PySys_Init () at sysmodule.c:413 #6 0x0002b414 in Py_Initialize () at pythonrun.c:142 #7 0x0001755c in Py_Main (argc=1, argv=0xffbffcdc) at main.c:245 #8 0x000171f0 in main (argc=1, argv=0xffbffcdc) at python.c:12 #8 0x000171f0 in main (argc=1, argv=0xffbffcdc) at python.c:12 (gdb) frame 1 #1 0x000304e0 in joinpath (buffer=0x83528 'A' <repeats 200 times>..., stuff=0x84140 'A' <repeats 200 times>...) at ./getpath.c:255 255 ./getpath.c: No such file or directory. in ./getpath.c snippet from Python-1.5.2/Modules/getpath.c : static void joinpath(buffer, stuff) char *buffer; char *stuff; { int n, k; if (stuff[0] == SEP) n = 0; else { n = strlen(buffer); if (n > 0 && buffer[n-1] != SEP && n < MAXPATHLEN) buffer[n++] = SEP; } k = strlen(stuff); if (n + k > MAXPATHLEN) k = MAXPATHLEN - n; strncpy(buffer+n, stuff, k); buffer[n+k] = '\0'; } further examining the backtrace: (gdb) print n 1=4999(gdb)printk1 = 4999 (gdb) print k 1=4999(gdb)printk2 = -3975 (gdb) print buffer 4=0x83528′A′<repeats200times>...(gdb)printstuff4 = 0x83528 'A' <repeats 200 times>... (gdb) print stuff 4=0x83528′A′<repeats200times>...(gdb)printstuff5 = 0x84140 'A' <repeats 200 times>... if (n + k > MAXPATHLEN) /* NOTE: MAXPATHLEN is 1024 */ k = MAXPATHLEN - n; /* NOTE: here k is 1024 - 4999 which is the -3975 */ Which of course crashes in strncpy(buffer+n, stuff, k); Thanks, Kirby Kuehl |
|
|
| msg21942 - (view) |
Author: Michael Hudson (mwh)  |
Date: 2004-08-07 15:55 |
| Logged In: YES user_id=6656 I'm not sure I understand. How do you get n to be so huge? At any rate, I don't think the bug is present in modern Python, but until I understand how this one was triggered, I'm not prepared to be sure... |
|
|
| msg21943 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-07 16:43 |
| Logged In: YES user_id=31435 Pretty compilcated internally. A very similar joinpath still exists. While it's not documented or checked, joinpath's code clearly *assumes* strlen(buffer) <= MAXPATHLEN on entry. But it's called from 25 places, and it's not immediately obvious that all call sites guarantee this on all paths. Rev 1.32 of getpath (for Python 2.0) *intended* to fix buffer overflow problems, mostly by changing unsafe strcpy() calls to strncpy() calls. This is delicate, though. I'd be a lot happier if joinpath verified that n <= MAXPATHLEN on entry, and called Py_FatalError() if not so (converting a buffer overrun into a bug report). |
|
|
| msg21944 - (view) |
Author: Kirby Kuehl (vacuum) |
Date: 2004-08-07 18:15 |
| Logged In: YES user_id=116409 by exporting environment variables. Check sharefuzz which can be found here: http://www.atstake.com/research/tools/index.html#vulnerability_scanning |
|
|
| msg21945 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-07 18:39 |
| Logged In: YES user_id=31435 Yup, and rev 1.32 intended to plug the envar attack. |
|
|
| msg21946 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-07 18:44 |
| Logged In: YES user_id=31435 I'm going to add the panic-check I suggested -- this code is too complicated to have any confidence in "eyeball analysis" over time. |
|
|
| msg21947 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-07 19:13 |
| Logged In: YES user_id=31435 I checked in the change to PC/getpathp.c, which is used on Windows. I'm attaching a patch to Modules/getpath.c, which isn't used on Windows (so I can't test it). |
|
|
| msg21948 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-07 19:15 |
| Logged In: YES user_id=31435 Unassigned myself, since I did all I can here. Someone on Linux should test the patch I attached. |
|
|
| msg21949 - (view) |
Author: Kirby Kuehl (vacuum) |
Date: 2004-08-07 22:19 |
| Logged In: YES user_id=116409 As tim_one poin |
|
|
| msg21950 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-08 01:01 |
| Logged In: YES user_id=31435 I checked in the attached patch too. If there's ever another release in the 2.3 line, these patches would make decent backport candidates. |
|
|
| msg21951 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2004-08-09 17:39 |
| Logged In: YES user_id=11375 How would I go about testing the patch? Where can I find code that tickles the bug? |
|
|
| msg21952 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-09 17:56 |
| Logged In: YES user_id=31435 The patch didn't intend to fix a bug that's known to exist. Python 2.0 presumably fixed the bugs here, but in a shaky way, relying on dozens of distinct call sites to establish an undocumented precondition. joinpath() suffers a buffer overrun if the precondition isn't met. The patch verifies that the precondition *is* met, killing the Python process if it's not. That should never happen, but joinpath() is called on so many distinct code paths that "eyeball analysis" is inadequate. To test the patch, change any of joinpath's call sites to violate the precondition, and see that Python dies then. The OP suggested a driver to provoke an envar attack, in the 2004-08-07 14:15 comment. |
|
|
| msg21953 - (view) |
Author: Kirby Kuehl (vacuum) |
Date: 2004-08-09 18:26 |
| Logged In: YES user_id=116409 Use the sharefuzz utility that I referenced in another comment. Then call any function that will invoke the joinpath function call. |
|
|
| msg21954 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-08-09 18:43 |
| Logged In: YES user_id=31435 vacuum, are you saying that sharefuzz provokes an actual bug in Python 2.4a2? In Python 2.3.4? |
|
|
| msg21955 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2004-10-19 20:10 |
| Logged In: YES user_id=11375 The URL mentioned for sharefuzz seems to be out of date; the correct location is now http://sourceforge.net/projects/sharefuzz/ . |
|
|
| msg21956 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2004-10-19 20:18 |
| Logged In: YES user_id=11375 I haven't figured out how to get sharefuzz to work yet, but did notice one OS/2 specific overflow; around line 3250 of posixmodule.c, it defines char args[1024], and then does a strcpy from the COMSPEC env. var. into args. |
|
|
| msg21957 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2004-10-19 20:41 |
| Logged In: YES user_id=11375 OK; I think I've got it working now, and neither 2.4CVS or 2.3-maint run into any problems with the sharefuzz library in use. I'm trying to pull the r234 tag, but SF CVS is being really slow; I'll try it tomorrow. |
|
|
| msg21958 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2004-10-20 12:03 |
| Logged In: YES user_id=11375 CVS finally came through, and r234 doesn't seem to report any problems; I put a debug print into joinpath() so I'm sure that it was actually being invoked. So I think this bug no longer applies, in the absence of a transcript demonstrating an actual problem. I'll reassign this bug to Andrew MacIntyre so that he knows about the OS/2 overflow and can fix it before his next OS/2 release. |
|
|
| msg21959 - (view) |
Author: Andrew I MacIntyre (aimacintyre) *  |
Date: 2004-12-12 08:43 |
| Logged In: YES user_id=250749 OS/2 specific fixes checked in. As there appears to be no further action required, I'm closing this item. |
|
|