homepage

The following script exploits a comment in funcobject.c:
 /* XXX This is broken if the caller deletes dict items! */
Because the code only borrows references to the items, it is possible to
have them destroyed before they are copied into the called frame.
class Name(str):
 def __eq__(self, other):
 del x[self]
 return str.__eq__(self, other)
 def __hash__(self):
 return str.__hash__(self)
x = {Name("a"):1, Name("b"):2}
def f(a, b): print a,b
f(**x) # Segfault

It also appears that the tuple and dict checks in function_call are
redundant. If there is a use case that results in function_call getting
anything (non null) other than dict and tuple in arg and kw, I don't
think silently ignoring such arguments is the right behavior.

This could be turned into an assertion.
But beware that some extension writers may use code like
 PyObject_Call(callable, args, Py_None)
which works perfectly today.

According to http://docs.python.org/api/object.html,
"""
PyObject* PyObject_Call(	PyObject *callable_object, PyObject *args,
PyObject *kw)
 Return value: New reference.
 Call a callable Python object callable_object, with arguments given
by the tuple args, and named arguments given by the dictionary kw. If no
named arguments are needed, kw may be NULL. args must not be NULL, use
an empty tuple if no arguments are needed. Returns the result of the
call on success, or NULL on failure. 
"""
passing Py_None as kw is not allowed. Interestingly, the documentation
also says that "args must not be NULL," while the code is clearly more
forgiving.

Sorry, please ignore my last comment about arg. The null and tuple
checks are for argdefs, not arg.

I have a proposed minimal fix, but I wonder if this would break existing
code (apart from the exploit given here). Martin? (I think the
discussion between Alexander and Amaury can be ignored for the purpose
of reviewing the fix; only the exploit matters.)

I think we agree that this patch has the potential of breaking existing
valid code. So based on the policy that we should avoid doing so in a
bugfix release, I'd rather reject that fix (fix2016.txt) for 2.5.x.
OTOH, if it is really unlikely that is ever occurs in existing code,
there would be no point in backporting it to 2.5.x, since the check
wouldn't trigger. 
I also can't see a security concern - applications shouldn't pass
untrusted objects as keyword arguments (if they were, such objects could
 put their malicious code inside __hash__).

I am attaching my fix along the lines of a solution suggested by Amaury 
at http://mail.python.org/pipermail/python-dev/2008-
February/076747.html:
"""
> Or is the proper fix to incref the values
> going into the kw array and decref them upon exit?
Yet Another Kind Of Tuple... However this seems the correct thing to do.
"""
I did not do any performance tests with this patch.

Confirmed in py3k (debug), trunk (non-debug) hangs with lots of CPU
activity.
py3k backtrace:
#0 0x0805c196 in do_richcompare (v=0x83e734c, w=0xb7afa648, op=2) at
Objects/object.c:561
#1 0x0805c450 in PyObject_RichCompare (v=0x83e734c, w=0xb7afa648, op=2)
at Objects/object.c:611
#2 0x0805c4f8 in PyObject_RichCompareBool (v=0x83e734c, w=0xb7afa648,
op=2) at Objects/object.c:633
#3 0x080b4140 in PyEval_EvalCodeEx (co=0x836b748, globals=0xb7d4e8f4,
locals=0x0, args=0xb7d14048, argcount=0,
 kws=0xb7ac1c08, kwcount=2, defs=0x0, defcount=0, kwdefs=0x0,
closure=0x0) at Python/ceval.c:3019
#4 0x08168f6c in function_call (func=0x839fa84, arg=0xb7d14034,
kw=0x82998f4) at Objects/funcobject.c:628
#5 0x08136e66 in PyObject_Call (func=0x839fa84, arg=0xb7d14034,
kw=0x82998f4) at Objects/abstract.c:2161
#6 0x080b74d5 in ext_do_call (func=0x839fa84, pp_stack=0xbfe58674,
flags=2, na=0, nk=0) at Python/ceval.c:4045
#7 0x080b1ae7 in PyEval_EvalFrameEx (f=0x828b57c, throwflag=0) at
Python/ceval.c:2571
#8 0x080b49fe in PyEval_EvalCodeEx (co=0x8370da8, globals=0xb7d4e8f4,
locals=0xb7d4e8f4, args=0x0, argcount=0, kws=0x0,
 kwcount=0, defs=0x0, defcount=0, kwdefs=0x0, closure=0x0) at
Python/ceval.c:3180
#9 0x080a901d in PyEval_EvalCode (co=0x8370da8, globals=0xb7d4e8f4,
locals=0xb7d4e8f4) at Python/ceval.c:650
#10 0x080e269c in run_mod (mod=0x84112f0, filename=0x81bef10 "<stdin>",
globals=0xb7d4e8f4, locals=0xb7d4e8f4,
 flags=0xbfe58ab8, arena=0x8348338) at Python/pythonrun.c:1697
#11 0x080e08d7 in PyRun_InteractiveOneFlags (fp=0xb7e9e440,
filename=0x81bef10 "<stdin>", flags=0xbfe58ab8)
 at Python/pythonrun.c:1091
#12 0x080e03dc in PyRun_InteractiveLoopFlags (fp=0xb7e9e440,
filename=0x81bef10 "<stdin>", flags=0xbfe58ab8)
 at Python/pythonrun.c:993
#13 0x080e0234 in PyRun_AnyFileExFlags (fp=0xb7e9e440,
filename=0x81bef10 "<stdin>", closeit=0, flags=0xbfe58ab8)
 at Python/pythonrun.c:962
#14 0x080f7fc4 in Py_Main (argc=1, argv=0xb7d12028) at Modules/main.c:625
#15 0x0805b22e in main (argc=1, argv=0xbfe59be4) at ./Modules/python.c:71
trunk backtrace:
Program received signal SIGINT, Interrupt.
[Switching to Thread 0xb7da68c0 (LWP 22330)]
0x080ff6a8 in PyTraceBack_Print (v=0xb7bf1e3c, f=0xb7d780c0) at
Python/traceback.c:243
243 while (tb1 != NULL) {
(gdb) bt
#0 0x080ff6a8 in PyTraceBack_Print (v=0xb7bf1e3c, f=0xb7d780c0) at
Python/traceback.c:243
#1 0x080f6f56 in PyErr_Display (exception=0x816a540, value=0xb7bf504c,
tb=0xb7bf1e3c) at Python/pythonrun.c:1200
#2 0x080fd7ed in sys_excepthook (self=0x0, args=0xb7bf1d4c) at
Python/sysmodule.c:138
#3 0x0805e8e5 in PyObject_Call (func=0xb7d7728c, arg=0xb7bf1d4c,
kw=0x0) at Objects/abstract.c:2506
#4 0x080cf882 in PyEval_CallObjectWithKeywords (func=0xb7d7728c,
arg=0xb7bf1d4c, kw=0x0) at Python/ceval.c:3781
#5 0x080f78ce in PyErr_PrintEx (set_sys_last_vars=1) at
Python/pythonrun.c:1146
#6 0x080f7f9c in PyRun_InteractiveOneFlags (fp=0xb7ef2440,
filename=0x8144910 "<stdin>", flags=0xbfe8a348)
 at Python/pythonrun.c:1037
#7 0x080f80f6 in PyRun_InteractiveLoopFlags (fp=0xb7ef2440,
filename=0x8144910 "<stdin>", flags=0xbfe8a348)
 at Python/pythonrun.c:763
#8 0x080f89e2 in PyRun_AnyFileExFlags (fp=0xb7ef2440,
filename=0x8144910 "<stdin>", closeit=0, flags=0xbfe8a348)
 at Python/pythonrun.c:732
#9 0x0805ac4e in Py_Main (argc=0, argv=0xbfe8a414) at Modules/main.c:599
#10 0x08059eb2 in main (argc=Cannot access memory at address 0x1
) at ./Modules/python.c:23

Fixed in trunk with r73564.
I performed performance tests: differences with pybench were negligible 
(<1%), but a specially crafted case like:
 kw = dict(a=1, b=2, c=3)
 for x in xrange(self.rounds):
 f(**kw)
showed an improvement of 21%!
Will backport to various branches after 3.1 is out.

Is everything fixed, so this can be closed?

It was merged to py3k in r73623, 3.1 in r73625, but not, as far as I can see, to 2.6.

Can someone please backport this to 2.7 so we can get this closed, thanks.

The fix was applied to trunk before the creation of the 2.7 branch.
There is nothing to backport

On Mon, Sep 20, 2010 at 10:23 AM, Mark Lawrence <report@bugs.python.org> wrote:
>
> Mark Lawrence <breamoreboy@yahoo.co.uk> added the comment:
>
> Can someone please backport this to 2.7 so we can get this closed, thanks.
>
AFAICT, r73564 preceded 2.7 branch cut, so the fix is already in 2.7.