Issue3664
Created on 2008-08-24 20:26 by ajaksu2, last changed 2008-10-17 20:16 by amaury.forgeotdarc.
| File name |
Uploaded |
Description |
Edit |
Remove |
|
_pickle.c
|
alexandre.vassalotti,
2008-10-04 22:54
|
|
|
|
|
issue3664_fix.diff
|
alexandre.vassalotti,
2008-10-17 05:10
|
|
|
|
|
msg71860 - (view) |
Author: Daniel Diniz (ajaksu2) |
Date: 2008-08-24 20:26 |
|
This script segfaults:
##
import _pickle
obj = _pickle.Pickler(open("/bin/ls")) #can be open(__file__) for scripts
try: obj.__init__('pouet', 87)
except Exception as err: pass
obj.dump(0)
###
[Switching to Thread -1210775360 (LWP 19096)]
0xb79fbf91 in pickler_write (self=0xb7a2fe4c, s=0xbff441a1 "...", n=2)
at /home/ajaksu/py3k/Modules/_pickle.c:442
442 memcpy(self->write_buf + self->buf_size, s, n);
(gdb) backtrace
#0 0xb79fbf91 in pickler_write (self=0xb7a2fe4c, s=0xbff441a1 "...",
n=2) at /home/ajaksu/py3k/Modules/_pickle.c:442
#1 0xb7a00a8c in dump (self=0xb7a2fe4c, obj=0x81fbd78) at
/home/ajaksu/py3k/Modules/_pickle.c:2288
#2 0xb7a00bb8 in Pickler_dump (self=0xb7a2fe4c, args=0xb7b30034) at
/home/ajaksu/py3k/Modules/_pickle.c:2328
#3 0x081626b1 in PyCFunction_Call (func=0xb796c3ec, arg=0xb7b30034,
kw=0x0) at Objects/methodobject.c:81
#4 0x080b378f in call_function (pp_stack=0xbff442e4, oparg=1) at
Python/ceval.c:3403
#5 0x080ae8d2 in PyEval_EvalFrameEx (f=0x829bafc, throwflag=0) at
Python/ceval.c:2205
#6 0x080b1c24 in PyEval_EvalCodeEx (co=0xb7acf2c8, globals=0xb7a9a8f4,
locals=0xb7a9a8f4, args=0x0, argcount=0, kws=0x0,
kwcount=0, defs=0x0, defcount=0, kwdefs=0x0, closure=0x0) at
Python/ceval.c:2840
Found using Fusil.
|
|
msg71863 - (view) |
Author: Christian Heimes (christian.heimes) |
Date: 2008-08-24 20:44 |
|
pickler_write() has no check for self->write_buf == NULL
Suggested patch:
===================================================================
--- Modules/_pickle.c (Revision 66010)
+++ Modules/_pickle.c (Arbeitskopie)
@@ -421,6 +421,10 @@
{
PyObject *data, *result;
+ if (self->write_buf == NULL) {
+ PyErr_SetString(PyExc_SystemError, "Invalid write buffer");
+ return -1;
+ }
if (s == NULL) {
if (!(self->buf_size))
return 0;
|
|
msg71938 - (view) |
Author: Alexandre Vassalotti (alexandre.vassalotti) |
Date: 2008-08-25 15:25 |
|
Oh, that's nasty. Recalling __init__ with bad arguments breaks the
internal invariants as it clears the Pickler's content before parsing
the arguments. I suspect that Unpickler is vulnerable too.
Adding a NULL check in pickler_write will only fix this particular
example. I could probably find another crash example that doesn't use
pickler_write.
|
|
msg71942 - (view) |
Author: Alexandre Vassalotti (alexandre.vassalotti) |
Date: 2008-08-25 16:00 |
|
Unpickler looks safe as Unpickler_load() checks if Unpickler was
properly initialized. And only Pickler_dump is vulnerable right now (new
methods, if any, exposed for issue3385 will have to take into account
this vulnerability).
|
|
msg74164 - (view) |
Author: Alexandre Vassalotti (alexandre.vassalotti) |
Date: 2008-10-02 14:36 |
|
I will try to find time next weekend to fix this (and other pickle
blockers).
|
|
msg74327 - (view) |
Author: Alexandre Vassalotti (alexandre.vassalotti) |
Date: 2008-10-04 22:54 |
|
Here's the fix. The added check in Pickler_dump should prevent any
segfaults due to __init__() errors.
I also added the check proposed by Christian as a safe-guard in case a
core developer adds a new method that doesn't check if the object was
properly initialized.
|
|
msg74888 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2008-10-17 01:34 |
|
Rather than attach a full _pickle.c file, please generate a unified diff
with just your changes. The patch should include a test for the
crashing condition. If you can upload that I'll try to accept it for
rc3. Deferring for now.
|
|
msg74896 - (view) |
Author: Alexandre Vassalotti (alexandre.vassalotti) |
Date: 2008-10-17 05:10 |
|
Oops. I must have been quite tired when I submitted that.
Here's the patch for the fix and the test case.
|
|
msg74898 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-10-17 07:48 |
|
The patch is fine.
|
|
msg74902 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2008-10-17 11:52 |
|
Amaury, please apply the patch and close the issue. Thanks!
|
|
msg74936 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-10-17 20:16 |
|
Committed r66963.
|
|
| Date |
User |
Action |
Args |
| 2008-10-17 20:16:14 | amaury.forgeotdarc | set | status: open -> closed resolution: accepted -> fixed messages:
+ msg74936 |
| 2008-10-17 11:52:42 | barry | set | priority: deferred blocker -> release blocker messages:
+ msg74902 |
| 2008-10-17 07:49:00 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc resolution: accepted messages:
+ msg74898 |
| 2008-10-17 05:10:11 | alexandre.vassalotti | set | files:
+ issue3664_fix.diff keywords:
+ patch messages:
+ msg74896 |
| 2008-10-17 01:34:45 | barry | set | priority: release blocker -> deferred blocker nosy:
+ barry messages:
+ msg74888 |
| 2008-10-04 22:54:16 | alexandre.vassalotti | set | files:
+ _pickle.c messages:
+ msg74327 |
| 2008-10-02 20:13:59 | jcea | set | nosy:
+ jcea |
| 2008-10-02 14:36:27 | alexandre.vassalotti | set | messages:
+ msg74164 |
| 2008-10-02 14:36:15 | alexandre.vassalotti | set | messages:
- msg74163 |
| 2008-10-02 14:35:52 | alexandre.vassalotti | set | messages:
+ msg74163 |
| 2008-10-02 12:52:55 | barry | set | priority: deferred blocker -> release blocker |
| 2008-09-26 22:17:25 | barry | set | priority: release blocker -> deferred blocker |
| 2008-09-18 05:41:57 | barry | set | priority: deferred blocker -> release blocker |
| 2008-09-04 01:15:19 | benjamin.peterson | set | priority: release blocker -> deferred blocker |
| 2008-08-25 16:00:03 | alexandre.vassalotti | set | messages:
+ msg71942 |
| 2008-08-25 15:25:45 | alexandre.vassalotti | set | messages:
+ msg71938 |
| 2008-08-24 20:44:11 | christian.heimes | set | nosy:
+ christian.heimes messages:
+ msg71863 |
| 2008-08-24 20:27:45 | benjamin.peterson | set | priority: release blocker assignee: alexandre.vassalotti nosy:
+ alexandre.vassalotti |
| 2008-08-24 20:26:34 | ajaksu2 | create | |
|