This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Crash when modifying the **kwargs passed to a function.
Type: crash Stage: commit review
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: amaury.forgeotdarc Nosy List: BreamoreBoy, ajaksu2, amaury.forgeotdarc, belopolsky, gregory.p.smith, gvanrossum, loewis, r.david.murray, terry.reedy
Priority: normal Keywords: patch

Created on 2008-02-06 01:01 by amaury.forgeotdarc, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fix2016.txt gvanrossum, 2008-02-11 18:13 use PyString_CheckExact for keyword names.
issue2016.diff belopolsky, 2008-03-31 15:25 Patch against revision 62083
Messages (15)
msg62086 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-02-06 01:01
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
msg62104 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-06 16:43
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.
msg62105 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-02-06 17:08
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.
msg62107 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-06 17:52
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.
msg62109 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-06 17:57
Sorry, please ignore my last comment about arg.  The null and tuple
checks are for argdefs, not arg.
msg62290 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-02-11 18:13
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.)
msg62325 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-02-12 18:40
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__).
msg64785 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-31 15:25
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.
msg86588 - (view) Author: Daniel Diniz (ajaksu2) * (Python triager) Date: 2009-04-26 01:45
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
msg89716 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-06-25 22:37
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.
msg104548 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-04-29 17:57
Is everything fixed, so this can be closed?
msg104560 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-04-29 18:24
It was merged to py3k in r73623, 3.1 in r73625, but not, as far as I can see, to 2.6.
msg116943 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-09-20 14:23
Can someone please backport this to 2.7 so we can get this closed, thanks.
msg116945 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-09-20 14:33
The fix was applied to trunk before the creation of the 2.7 branch.
There is nothing to backport
msg116946 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-09-20 14:34
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.
History
Date User Action Args
2022-04-11 14:56:30adminsetgithub: 46300
2010-09-20 14:34:10belopolskysetmessages: + msg116946
2010-09-20 14:33:48amaury.forgeotdarcsetstatus: open -> closed

messages: + msg116945
2010-09-20 14:23:14BreamoreBoysetnosy: + BreamoreBoy

messages: + msg116943
versions: + Python 2.7, - Python 2.6
2010-04-29 18:24:12r.david.murraysetversions: - Python 2.5, Python 3.0, Python 3.1, Python 2.7
nosy: + r.david.murray

messages: + msg104560

stage: patch review -> commit review
2010-04-29 17:57:50terry.reedysetstatus: pending -> open
nosy: + terry.reedy
messages: + msg104548

2009-06-25 22:37:43amaury.forgeotdarcsetstatus: open -> pending
assignee: amaury.forgeotdarc
resolution: fixed
messages: + msg89716
2009-04-26 01:45:35ajaksu2setstage: patch review
messages: + msg86588
versions: + Python 3.0, Python 3.1, Python 2.7
2008-09-08 11:38:00ajaksu2setnosy: + ajaksu2
2008-03-31 15:25:38belopolskysetfiles: + issue2016.diff
keywords: + patch
messages: + msg64785
2008-02-12 18:40:40loewissetpriority: high -> normal
assignee: loewis -> (no value)
messages: + msg62325
2008-02-11 18:13:52gvanrossumsetfiles: + fix2016.txt
assignee: loewis
messages: + msg62290
nosy: + gvanrossum, loewis
2008-02-10 04:51:44gregory.p.smithsetnosy: + gregory.p.smith
2008-02-06 17:57:13belopolskysetmessages: + msg62109
2008-02-06 17:52:23belopolskysetmessages: + msg62107
2008-02-06 17:08:00amaury.forgeotdarcsetmessages: + msg62105
2008-02-06 16:43:17belopolskysetnosy: + belopolsky
messages: + msg62104
2008-02-06 12:07:24christian.heimessetpriority: high
components: + Interpreter Core
2008-02-06 01:01:05amaury.forgeotdarccreate