From e0d478936c2288957e004bf2853e24f78502a867 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Mon, 22 May 2017 13:06:30 +0100 Subject: [PATCH 1/5] Protect against infinite loops in inspect.unwrap() --- Lib/inspect.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 9c072eb0747fbd..da97533c844197 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -506,10 +506,13 @@ def _is_wrapper(f): return hasattr(f, '__wrapped__') and not stop(f) f = func # remember the original func for error reporting memo = {id(f)} # Memoise by id to tolerate non-hashable objects + unwrap_count = 0 + recursion_limit = sys.getrecursionlimit() while _is_wrapper(func): func = func.__wrapped__ id_func = id(func) - if id_func in memo: + unwrap_count += 1 + if (id_func in memo) or (unwrap_count >= recursion_limit): raise ValueError('wrapper loop when unwrapping {!r}'.format(f)) memo.add(id_func) return func From 87c0c0e4a93c6cc284f8295a690f58b925461d8a Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Mon, 22 May 2017 14:46:11 +0100 Subject: [PATCH 2/5] Add note about bpo-25532 in NEWS --- Misc/NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index 74f7922934d90c..465334ce2ec407 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -989,6 +989,10 @@ Library - Issue #29581: ABCMeta.__new__ now accepts ``**kwargs``, allowing abstract base classes to use keyword parameters in __init_subclass__. Patch by Nate Soares. +- Issue #25532: inspect.unwrap() will now only try to unwrap an object + sys.getrecursionlimit() times, to protect against objects which create a new + object on every attribute access. + Windows ------- From 2cb0a18450ef5e634123eb4539cc24961a1e02e7 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Mon, 22 May 2017 15:53:32 +0100 Subject: [PATCH 3/5] Add test for infinite loop protection in inspect.unwrap() --- Lib/test/test_inspect.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index c55efd69425173..350d5dbd776ac2 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -3554,6 +3554,19 @@ def test_builtins_have_signatures(self): self.assertIsNone(obj.__text_signature__) +class NTimesUnwrappable: + def __init__(self, n): + self.n = n + self._next = None + + @property + def __wrapped__(self): + if self.n <= 0: + raise Exception("Unwrapped too many times") + if self._next is None: + self._next = NTimesUnwrappable(self.n - 1) + return self._next + class TestUnwrap(unittest.TestCase): def test_unwrap_one(self): @@ -3609,6 +3622,11 @@ class C: __wrapped__ = func self.assertIsNone(inspect.unwrap(C())) + def test_recursion_limit(self): + obj = NTimesUnwrappable(sys.getrecursionlimit() + 1) + with self.assertRaisesRegex(ValueError, 'wrapper loop'): + inspect.unwrap(obj) + class TestMain(unittest.TestCase): def test_only_source(self): module = importlib.import_module('unittest') From b12a0ac8e640c63b5a52a8319a4064743935d198 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Mon, 22 May 2017 20:08:10 +0100 Subject: [PATCH 4/5] Change memo set to a dict --- Lib/inspect.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index da97533c844197..da6e50097a795b 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -505,7 +505,9 @@ def _is_wrapper(f): def _is_wrapper(f): return hasattr(f, '__wrapped__') and not stop(f) f = func # remember the original func for error reporting - memo = {id(f)} # Memoise by id to tolerate non-hashable objects + # Memoise by id to tolerate non-hashable objects, but store objects to + # ensure they aren't destroyed, which would allow their IDs to be reused. + memo = {id(f): f} unwrap_count = 0 recursion_limit = sys.getrecursionlimit() while _is_wrapper(func): @@ -514,7 +516,7 @@ def _is_wrapper(f): unwrap_count += 1 if (id_func in memo) or (unwrap_count >= recursion_limit): raise ValueError('wrapper loop when unwrapping {!r}'.format(f)) - memo.add(id_func) + memo[id_func] = func return func # -------------------------------------------------- source code extraction From 553df2589559645a617faeeb2364cfa6aab8f6ef Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Mon, 22 May 2017 23:46:21 +0100 Subject: [PATCH 5/5] Use len(memo) instead of a separate unwrap_count --- Lib/inspect.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index da6e50097a795b..9a843d6420e2e0 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -508,13 +508,11 @@ def _is_wrapper(f): # Memoise by id to tolerate non-hashable objects, but store objects to # ensure they aren't destroyed, which would allow their IDs to be reused. memo = {id(f): f} - unwrap_count = 0 recursion_limit = sys.getrecursionlimit() while _is_wrapper(func): func = func.__wrapped__ id_func = id(func) - unwrap_count += 1 - if (id_func in memo) or (unwrap_count >= recursion_limit): + if (id_func in memo) or (len(memo) >= recursion_limit): raise ValueError('wrapper loop when unwrapping {!r}'.format(f)) memo[id_func] = func return func