gh-142781: Fix type confusion in zoneinfo weak cache #142925
Conversation
Validate the types returned from _weak_cache.get() and _weak_cache.setdefault() to prevent type confusion when a ZoneInfo subclass provides a misbehaving cache implementation.
|
All commit authors signed the Contributor License Agreement. |
| } | ||
|
|
||
| if (instance != Py_None && !PyObject_TypeCheck(instance, type)) { | ||
| const char *e = "%s._weak_cache.get() returned %s, expected %s"; |
There was a problem hiding this comment.
No need to have a temporary variable. In addition, avoid exposing implementation details about weak_cache here. Instead, I would simply say that the instance is incorrect (something like "expecting a %T, got %T")
| Fix a type confusion in zoneinfo where a malicious override of _weak_cache | ||
| in a zoneinfo subclass can cause memory corruption and crash the | ||
| interpreter. |
There was a problem hiding this comment.
This entry should be rewritten, it doesn't help for the end user. It's not necessarily a "malicious" override, it could be accidental as well. I would rather have something like
:mod:`zoneinfo`: fix a crash when instantiating :class:`~zoneinfo.Zoneinfo`
objects for which the internal class-level cache is consistent.| class CZoneInfoCacheTest(ZoneInfoCacheTest): | ||
| module = c_zoneinfo | ||
|
|
||
| def test_weak_cache_get_type_confusion(self): |
There was a problem hiding this comment.
| def test_weak_cache_get_type_confusion(self): | |
| def test_inconsistent_weak_cache_get(self): |
| with self.assertRaises(TypeError): | ||
| EvilZoneInfo("America/Los_Angeles") | ||
|
|
||
| def test_weak_cache_setdefault_type_confusion(self): |
There was a problem hiding this comment.
| def test_weak_cache_setdefault_type_confusion(self): | |
| def test_inconsistent_weak_cache_setdefault(self): |
| EvilZoneInfo("America/Los_Angeles") | ||
|
|
||
| def test_weak_cache_setdefault_type_confusion(self): | ||
| class EvilCache: |
There was a problem hiding this comment.
Rename EvilCache with Cache and EvilZoneInfo by ZI only. In addition, create _weak_cache when instantiating the class:
class ZI(self.class):
_weak_cache = Cache()Then, check the TypeError message.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
2 participants
Validate the types returned from
_weak_cache.get()and_weak_cache.setdefault()to prevent type confusion when a ZoneInfo subclass provides a misbehaving cache implementation.Fixes gh-142781.