Issue3664
Created on 2008-08-24 20:26 by ajaksu2, last changed 2022-04-11 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| _pickle.c | alexandre.vassalotti, 2008-10-04 22:54 | |||
| issue3664_fix.diff | alexandre.vassalotti, 2008-10-17 05:10 | |||
| Messages (11) | |||
|---|---|---|---|
| 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. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:38 | admin | set | github: 47914 |
| 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 | |

