From 249565f696ae35e4e0842c55c7a88304fb0e00c5 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Fri, 27 Jun 2025 20:10:14 +0000 Subject: [PATCH 01/16] gh-136053: Memory Safety Issue in marshal.c TYPE_SLICE Case --- Python/marshal.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Python/marshal.c b/Python/marshal.c index afbef6ee6796d9..2fca6d2dcf2848 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1658,7 +1658,14 @@ r_object(RFILE *p) Py_ssize_t idx = r_ref_reserve(flag, p); PyObject *stop = NULL; PyObject *step = NULL; - PyObject *start = r_object(p); + PyObject *start = NULL; + + if (idx < 0 && flag) { + // r_ref_reserve failed + break; + } + + start = r_object(p); if (start == NULL) { goto cleanup; } @@ -1671,7 +1678,9 @@ r_object(RFILE *p) goto cleanup; } retval = PySlice_New(start, stop, step); - r_ref_insert(retval, idx, flag, p); + if (retval != NULL) { + r_ref_insert(retval, idx, flag, p); + } cleanup: Py_XDECREF(start); Py_XDECREF(stop); From 7c95d8f323c6b55e54d74072f50a86810ba23279 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 20:14:13 +0000 Subject: [PATCH 02/16] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst b/Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst new file mode 100644 index 00000000000000..fd5f0dec519e7f --- /dev/null +++ b/Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst @@ -0,0 +1 @@ +Fix a potential out-of-bounds memory access vulnerability in the ``TYPE_SLICE`` case of the ``r_object`` function in ``marshal.c``. The code now properly checks if ``r_ref_reserve()`` fails before using its return value, preventing crashes or security issues when deserializing malicious marshal data. From 818304a47c2e4b6f25f4dce7aaad0ec4207d96cd Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 21:14:42 +0000 Subject: [PATCH 03/16] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst new file mode 100644 index 00000000000000..fd5f0dec519e7f --- /dev/null +++ b/Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst @@ -0,0 +1 @@ +Fix a potential out-of-bounds memory access vulnerability in the ``TYPE_SLICE`` case of the ``r_object`` function in ``marshal.c``. The code now properly checks if ``r_ref_reserve()`` fails before using its return value, preventing crashes or security issues when deserializing malicious marshal data. From 26d50fd7435861d6b54639d82f40859d6b9c36be Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 21:16:01 +0000 Subject: [PATCH 04/16] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst new file mode 100644 index 00000000000000..acd1357b40b51e --- /dev/null +++ b/Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst @@ -0,0 +1 @@ +:mod:``marshal``: fix a possible crash when deserializing :class:``slice`` objects. From 42edbea61b3bb774bf45f867d7e4ee6b8c1bb272 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Fri, 27 Jun 2025 21:16:21 +0000 Subject: [PATCH 05/16] Incorporating review comments --- Python/marshal.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Python/marshal.c b/Python/marshal.c index 2fca6d2dcf2848..a61a015750d256 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1660,8 +1660,7 @@ r_object(RFILE *p) PyObject *step = NULL; PyObject *start = NULL; - if (idx < 0 && flag) { - // r_ref_reserve failed + if (idx < 0) { break; } @@ -1678,9 +1677,6 @@ r_object(RFILE *p) goto cleanup; } retval = PySlice_New(start, stop, step); - if (retval != NULL) { - r_ref_insert(retval, idx, flag, p); - } cleanup: Py_XDECREF(start); Py_XDECREF(stop); From 23f29c2c150fbfa2efce84d6d20f147f5c7f5447 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Fri, 27 Jun 2025 21:18:25 +0000 Subject: [PATCH 06/16] Removing unnecessary rst --- .../next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst | 1 - .../next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst | 1 - 2 files changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst delete mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst b/Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst deleted file mode 100644 index fd5f0dec519e7f..00000000000000 --- a/Misc/NEWS.d/next/Security/2025-06-27-20-14-11.gh-issue-136053._pnEv0.rst +++ /dev/null @@ -1 +0,0 @@ -Fix a potential out-of-bounds memory access vulnerability in the ``TYPE_SLICE`` case of the ``r_object`` function in ``marshal.c``. The code now properly checks if ``r_ref_reserve()`` fails before using its return value, preventing crashes or security issues when deserializing malicious marshal data. diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst deleted file mode 100644 index fd5f0dec519e7f..00000000000000 --- a/Misc/NEWS.d/next/Security/2025-06-27-21-14-40.gh-issue-136053._pnEv0.rst +++ /dev/null @@ -1 +0,0 @@ -Fix a potential out-of-bounds memory access vulnerability in the ``TYPE_SLICE`` case of the ``r_object`` function in ``marshal.c``. The code now properly checks if ``r_ref_reserve()`` fails before using its return value, preventing crashes or security issues when deserializing malicious marshal data. From 5c654a3cdf771b87d2536165cb9fa261437b5969 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Fri, 27 Jun 2025 21:19:34 +0000 Subject: [PATCH 07/16] Removing extra line --- Python/marshal.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/marshal.c b/Python/marshal.c index a61a015750d256..f5cadf934fc67c 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1663,7 +1663,6 @@ r_object(RFILE *p) if (idx < 0) { break; } - start = r_object(p); if (start == NULL) { goto cleanup; From fca624c1aecc21720e62f6ba0459e336f1ddee46 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 21:22:29 +0000 Subject: [PATCH 08/16] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst new file mode 100644 index 00000000000000..acd1357b40b51e --- /dev/null +++ b/Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst @@ -0,0 +1 @@ +:mod:``marshal``: fix a possible crash when deserializing :class:``slice`` objects. From 9d500c329518fd937475798479a92c274cd2a300 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 21:23:20 +0000 Subject: [PATCH 09/16] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2025-06-27-21-23-19.gh-issue-136053.QZxcee.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-23-19.gh-issue-136053.QZxcee.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-23-19.gh-issue-136053.QZxcee.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-23-19.gh-issue-136053.QZxcee.rst new file mode 100644 index 00000000000000..93caed3aa3b9dd --- /dev/null +++ b/Misc/NEWS.d/next/Security/2025-06-27-21-23-19.gh-issue-136053.QZxcee.rst @@ -0,0 +1 @@ +:mod:`marshal`: fix a possible crash when deserializing :class:`slice` objects. From b17d057c01b5e86625f5ffb5a5268662e5153bbb Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Fri, 27 Jun 2025 21:23:48 +0000 Subject: [PATCH 10/16] Removing unnecessary rst --- .../next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst deleted file mode 100644 index acd1357b40b51e..00000000000000 --- a/Misc/NEWS.d/next/Security/2025-06-27-21-15-59.gh-issue-136053.KcyIsH.rst +++ /dev/null @@ -1 +0,0 @@ -:mod:``marshal``: fix a possible crash when deserializing :class:``slice`` objects. From 386d087eec5be3c02074ebc0cbefd8d2d4feaad3 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Fri, 27 Jun 2025 21:24:37 +0000 Subject: [PATCH 11/16] Removing unnecessary rst --- .../next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst diff --git a/Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst b/Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst deleted file mode 100644 index acd1357b40b51e..00000000000000 --- a/Misc/NEWS.d/next/Security/2025-06-27-21-22-28.gh-issue-136053.KcyIsH.rst +++ /dev/null @@ -1 +0,0 @@ -:mod:``marshal``: fix a possible crash when deserializing :class:``slice`` objects. From 17aadcb9c47a8980ab164a5abac1994eadcdd456 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Sat, 28 Jun 2025 04:14:52 +0000 Subject: [PATCH 12/16] Incorporating review comments --- Python/marshal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/marshal.c b/Python/marshal.c index f5cadf934fc67c..9c196a097cfbd9 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1659,7 +1659,6 @@ r_object(RFILE *p) PyObject *stop = NULL; PyObject *step = NULL; PyObject *start = NULL; - if (idx < 0) { break; } @@ -1676,6 +1675,7 @@ r_object(RFILE *p) goto cleanup; } retval = PySlice_New(start, stop, step); + r_ref_insert(retval, idx, flag, p); cleanup: Py_XDECREF(start); Py_XDECREF(stop); From 9aa277bfa4f24837ef8bbae19f0de37242474a34 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Sat, 28 Jun 2025 06:51:46 +0000 Subject: [PATCH 13/16] Moving up condition --- Python/marshal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/marshal.c b/Python/marshal.c index 9c196a097cfbd9..41d2e0d36dd097 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1656,12 +1656,12 @@ r_object(RFILE *p) case TYPE_SLICE: { Py_ssize_t idx = r_ref_reserve(flag, p); - PyObject *stop = NULL; - PyObject *step = NULL; - PyObject *start = NULL; if (idx < 0) { break; } + PyObject *stop = NULL; + PyObject *step = NULL; + PyObject *start = NULL; start = r_object(p); if (start == NULL) { goto cleanup; From 8e1804d2ceee505d5c932b0318679636572ba9fe Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Sat, 28 Jun 2025 06:53:01 +0000 Subject: [PATCH 14/16] Moving up condition --- Python/marshal.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/marshal.c b/Python/marshal.c index 41d2e0d36dd097..15dd25d6268df4 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -1661,8 +1661,7 @@ r_object(RFILE *p) } PyObject *stop = NULL; PyObject *step = NULL; - PyObject *start = NULL; - start = r_object(p); + PyObject *start = r_object(p); if (start == NULL) { goto cleanup; } From 4a744bb6bbf56cf7045a80b399db05467fcce985 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Sat, 28 Jun 2025 08:47:33 +0000 Subject: [PATCH 15/16] PoC: Adding test case --- Lib/test/test_marshal.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index 8b1fb0eba1f8b6..d11b919d2c2461 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -614,6 +614,27 @@ def test_slice(self): with self.assertRaises(ValueError): marshal.dumps(obj, version) + def test_slice_ref_reserve_failure(self): + # Test for the fix: if (idx < 0) { break; } + # This tests the case where r_ref_reserve fails when processing TYPE_SLICE with FLAG_REF + # We simulate a scenario where the reference list is too large + + # Create malformed marshal data: TYPE_SLICE with FLAG_REF but invalid reference handling + # This should trigger the r_ref_reserve failure path and be handled gracefully + malformed_data = b'\xba' # TYPE_SLICE | FLAG_REF (0x3a | 0x80) + malformed_data += b'N' # None for start + malformed_data += b'N' # None for stop + malformed_data += b'N' # None for step + + # This should raise an exception rather than crash + with self.assertRaises((ValueError, EOFError)): + marshal.loads(malformed_data) + + # Test truncated data that would also trigger the error path + truncated_data = b'\xba' + b'N' # TYPE_SLICE | FLAG_REF + only one component + with self.assertRaises((ValueError, EOFError)): + marshal.loads(truncated_data) + @support.cpython_only @unittest.skipUnless(_testcapi, 'requires _testcapi') class CAPI_TestCase(unittest.TestCase, HelperMixin): From cb246dfaf2f507ed50bb1101ce66e321e4a9a656 Mon Sep 17 00:00:00 2001 From: Akshat Gupta Date: Sat, 28 Jun 2025 12:11:07 +0000 Subject: [PATCH 16/16] Removing test --- Lib/test/test_marshal.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index d11b919d2c2461..8b1fb0eba1f8b6 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -614,27 +614,6 @@ def test_slice(self): with self.assertRaises(ValueError): marshal.dumps(obj, version) - def test_slice_ref_reserve_failure(self): - # Test for the fix: if (idx < 0) { break; } - # This tests the case where r_ref_reserve fails when processing TYPE_SLICE with FLAG_REF - # We simulate a scenario where the reference list is too large - - # Create malformed marshal data: TYPE_SLICE with FLAG_REF but invalid reference handling - # This should trigger the r_ref_reserve failure path and be handled gracefully - malformed_data = b'\xba' # TYPE_SLICE | FLAG_REF (0x3a | 0x80) - malformed_data += b'N' # None for start - malformed_data += b'N' # None for stop - malformed_data += b'N' # None for step - - # This should raise an exception rather than crash - with self.assertRaises((ValueError, EOFError)): - marshal.loads(malformed_data) - - # Test truncated data that would also trigger the error path - truncated_data = b'\xba' + b'N' # TYPE_SLICE | FLAG_REF + only one component - with self.assertRaises((ValueError, EOFError)): - marshal.loads(truncated_data) - @support.cpython_only @unittest.skipUnless(_testcapi, 'requires _testcapi') class CAPI_TestCase(unittest.TestCase, HelperMixin):