python
gh-143379: fix UAF in struct.Struct.pack() when object modified by dunder methods
Alternative approach: use some flag in PyStructObject struct to forbid mutation during pack().
Alternative approach: use some flag in PyStructObject struct to forbid mutation during pack().
Would it be possible to always disallow mutation? Raise an exception if __init__() is called twice.
Why there is __init__() method at all? The Struct expected to be an immutable type.
Maybe all logic in Struct___init__ (tp_init) should be moved to s_new (tp_new)? I think this will fix issue. Looking in the history, the custom __init__() was re-introduced back in #112358.
Here is the plan:
- create a flag to temporary forbid mutation
- deprecate
Struct.__new__()calls without one required argument - eventually drop
Struct.__init__()together with a flag
CC @serhiy-storchaka
Seen https://github.com/python/cpython/pull/143382#issuecomment-3712576525 after trying to review the code. I agree with @skirpichev, a flag is the right temporary solution, and we should think about getting rid of __init__() in future.
Ok, new version just disallows mutation of Struct() when s_pack_internal() is running. When support for __init__() will be dropped - the mutex field can be removed.
Edit: see #143643 for next steps.
Would not it prevent concurrent use of pack()?
Hmm, indeed.
Well, ignoring this issue until we forbid repeated calls of init() is also solution.
We can do this after a deprecation (see ongoing work in https://github.com/python/cpython/pull/143643). But then this will be a non-issue anymore. Then, maybe we can close the https://github.com/python/cpython/issues/143379 as a duplicate of https://github.com/python/cpython/issues/78724? The real problem here is that Struct()'s aren't immutable.
Ok, this lacks mutex in __init__() and benchmarks. But I'll not push things further. Thanks for review.
Oh, you closed your PR. Why?
See issue thread. I think that this not solves some practical problem, but introduce code complexity. If we make eventually make Struct immutable - https://github.com/python/cpython/issues/143379 will be fixed.