From cdae0a44b194bf3b8ced0246e009c3b95246d5d0 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 25 Sep 2024 14:43:28 +0200 Subject: [PATCH 1/3] gh-124513: Check args in framelocalsproxy_new() Fix a crash in FrameLocalsProxy constructor: check the number of arguments. --- Lib/test/test_frame.py | 17 +++++++++++++++++ ...24-09-25-14-45-56.gh-issue-124513.ywiXtr.rst | 2 ++ Objects/frameobject.c | 7 +++++++ 3 files changed, 26 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-09-25-14-45-56.gh-issue-124513.ywiXtr.rst diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index ca88e657367d9a..9a3aef664f34c5 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -494,6 +494,23 @@ class ObjectSubclass: with self.assertRaises(TypeError): proxy[obj] = 0 + def test_constructor(self): + FrameLocalsProxy = type([sys._getframe().f_locals + for x in range(1)][0]) + self.assertEqual(FrameLocalsProxy.__name__, 'FrameLocalsProxy') + + def make_frame(): + x = 1 + y = 2 + return sys._getframe() + + proxy = FrameLocalsProxy(make_frame()) + self.assertEqual(proxy, {'x': 1, 'y': 2}) + + # constructor expects 1 argument (frame) + with self.assertRaises(TypeError): + FrameLocalsProxy() + class FrameLocalsProxyMappingTests(mapping_tests.TestHashMappingProtocol): """Test that FrameLocalsProxy behaves like a Mapping (with exceptions)""" diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-09-25-14-45-56.gh-issue-124513.ywiXtr.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-09-25-14-45-56.gh-issue-124513.ywiXtr.rst new file mode 100644 index 00000000000000..691e03b3b98e7a --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-09-25-14-45-56.gh-issue-124513.ywiXtr.rst @@ -0,0 +1,2 @@ +Fix a crash in FrameLocalsProxy constructor: check the number of arguments. +Patch by Victor Stinner. diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 9f1c031dcb9a9d..8b786d9c6c1bcc 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -310,6 +310,13 @@ framelocalsproxy_dealloc(PyObject *self) static PyObject * framelocalsproxy_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { + if (PyTuple_GET_SIZE(args) != 1) { + PyErr_Format(PyExc_TypeError, + "FrameLocalsProxy expected 1 argument, got %zd", + PyTuple_GET_SIZE(args)); + return NULL; + } + PyFrameLocalsProxyObject *self = (PyFrameLocalsProxyObject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL; From 2dd64c93b662e9d1fe5d81fa6e576c95a095e317 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 25 Sep 2024 17:04:21 +0200 Subject: [PATCH 2/3] Check that the argument is a frame --- Lib/test/test_frame.py | 6 ++++-- Objects/frameobject.c | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index 9a3aef664f34c5..c37cdcc41fb49f 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -507,9 +507,11 @@ def make_frame(): proxy = FrameLocalsProxy(make_frame()) self.assertEqual(proxy, {'x': 1, 'y': 2}) - # constructor expects 1 argument (frame) + # constructor expects 1 frame argument with self.assertRaises(TypeError): - FrameLocalsProxy() + FrameLocalsProxy() # no arguments + with self.assertRaises(TypeError): + FrameLocalsProxy(123) # wrong type class FrameLocalsProxyMappingTests(mapping_tests.TestHashMappingProtocol): diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 8b786d9c6c1bcc..3ba926ceed00e6 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -316,15 +316,19 @@ framelocalsproxy_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PyTuple_GET_SIZE(args)); return NULL; } + PyObject *item = PyTuple_GET_ITEM(args, 0); + + if (!PyFrame_Check(item)) { + PyErr_Format(PyExc_TypeError, "expect frame, not %T", item); + return NULL; + } + PyFrameObject *frame = (PyFrameObject*)item; PyFrameLocalsProxyObject *self = (PyFrameLocalsProxyObject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL; } - PyFrameObject *frame = (PyFrameObject*)PyTuple_GET_ITEM(args, 0); - assert(PyFrame_Check(frame)); - ((PyFrameLocalsProxyObject*)self)->frame = (PyFrameObject*)Py_NewRef(frame); return (PyObject *)self; From 467b9cbf8907d6036f7c51779d67fdae76a4b6d3 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 25 Sep 2024 18:47:47 +0200 Subject: [PATCH 3/3] reject also keyword arguments --- Lib/test/test_frame.py | 2 ++ Objects/frameobject.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index c37cdcc41fb49f..32de8ed9a13f80 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -512,6 +512,8 @@ def make_frame(): FrameLocalsProxy() # no arguments with self.assertRaises(TypeError): FrameLocalsProxy(123) # wrong type + with self.assertRaises(TypeError): + FrameLocalsProxy(frame=sys._getframe()) # no keyword arguments class FrameLocalsProxyMappingTests(mapping_tests.TestHashMappingProtocol): diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 3ba926ceed00e6..f3a66ffc9aac8f 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -324,6 +324,12 @@ framelocalsproxy_new(PyTypeObject *type, PyObject *args, PyObject *kwds) } PyFrameObject *frame = (PyFrameObject*)item; + if (kwds != NULL && PyDict_Size(kwds) != 0) { + PyErr_SetString(PyExc_TypeError, + "FrameLocalsProxy takes no keyword arguments"); + return 0; + } + PyFrameLocalsProxyObject *self = (PyFrameLocalsProxyObject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL;