gh-149816: Fix a RC in _random.Random.__init__ method#149824
Open
sobolevn wants to merge 1 commit into
Open
Conversation
22 tasks
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am not sure that it would be easy to repro / test in a real env, but it can be a potential problem.
Original description from
17.md:Vulnerability #17
Title: Unlocked init races with PRNG state access
Category: Concurrency and Race Conditions
Tags: race,read,dos
CWEs: CWE-125, CWE-367
CVSS: CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:L/VI:N/VA:L/SC:N/SI:N/SA:N
Severity: Low (2.1)
Location:
cpython/Modules/_randommodule.c:572:572in functionrandom_initDescription
random_initreseeds internal MT state by callingrandom_seedwithout acquiring the object critical section atcpython/Modules/_randommodule.c:572, while normal methods usePy_BEGIN_CRITICAL_SECTION(self)(e.g.cpython/Modules/clinic/_randommodule.c.h:26,cpython/Modules/clinic/_randommodule.c.h:139). During reseed,init_genrandwritesself->index = Natcpython/Modules/_randommodule.c:212. A concurrent reader ingenrand_uint32can pass the bounds checkself->index >= Natcpython/Modules/_randommodule.c:143and then use a raced value atcpython/Modules/_randommodule.c:160, causingmt[624]access.Trigger Conditions
Pre-conditions:
Py_GIL_DISABLED); with the GIL enabled, the race is serialized._random.Randominstance is shared across threads.__init__andrandom()/getrandbits()on that same object (directly or through an exposed application API).Data flow:
r = _random.Random(0), then advance to a near-boundary index (e.g., callr.getrandbits(32)623 times so index becomes 623).r.getrandbits(32)and entersgenrand_uint32; it evaluatesself->index >= Nas false atcpython/Modules/_randommodule.c:143._random.Random.__init__(r, 1)(orr.__init__(1)), reaching unlockedrandom_initatcpython/Modules/_randommodule.c:572and theninit_genrandsettingself->index = 624atcpython/Modules/_randommodule.c:212.y = mt[self->index++]atcpython/Modules/_randommodule.c:160, reading out-of-bounds elementstate[624].Impact
In the free-threaded runtime, this race introduces undefined behavior and a concrete out-of-bounds read on PRNG object state. Most severe outcomes are process crash (availability impact) or unintended memory disclosure via corrupted/random output, depending on allocator/layout and timing. Exploitability requires precise concurrency and access to concurrent method invocation on the same object, which lowers reliability but does not remove the bug.
Remediation
random_init()(cpython/Modules/_randommodule.c), wrap therandom_seed(...)call in the same object critical section used by other mutating/accessor methods, so__init__cannot race withrandom(),getrandbits(),getstate(),setstate(), orseed()on the same instance.random_init()is exception-safe on all return paths (including failures), matching existing critical-section patterns used in generated wrappers.tp_inituse as well (the fix should apply regardless of whetherRandomis used directly or via subclasses reusing thistp_init).Lib/test(random module tests) that repeatedly racesr.__init__(...)againstr.random()/r.getrandbits(...)on the same object and asserts no crash/UB under stress.Reproduction
Py_GIL_DISABLED) with a sanitizer-enabled/debug configuration (ASAN or UBSAN strongly recommended) so races and out-of-bounds reads are visible instead of silently ignored.random.Randominstance and drive it close to the MT boundary state (consume enough outputs soindexis near623), then keep using that same object from multiple threads.random()orgetrandbits(32),obj.__init__(seed_value)(re-initializing the existing object, not creating a new one)._randommodule.c(notably aroundgenrand_uint32/ seed-init paths),__init__+random/getrandbitsworkload.Code Context