Repository navigation
CryptGenRandom says true when it worked - #317
Merged
Merged
Conversation
tamnd
added
area/runtime
The Windows arm of SecureRandomBytesGenerator::getRandomBytes tested the return value the wrong way round. CryptGenRandom returns a BOOL that is nonzero on success, so every successful call was reported as an error and every failure was reported as success. The first caller to trip over it is the telemetry code, which builds a session identifier out of 32 random bytes during startup, asserts that the call worked and takes the process down when it did not. That is why mojo build and mojo run both aborted on Windows before reading a single line of source. The failure arm was the worse half. A real failure left the buffer untouched and returned success, so the caller got whatever was on the stack and was told it was cryptographically secure. Nothing depends on that today beyond an identifier, but the function is named what it is named. Also put the Win32 error code in the message. There is nothing else to go on when this fails, since the call returns a bare false and the other two platforms either cannot fail or say why. Fixes #316
tamnd
force-pushed
the
random-inverted
branch
from
September 9, 2026 02:49
29fa96d to
3d53bd5
Compare
This was referenced Sep 9, 2026
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
1 participant
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.
Fixes #316.
mojo buildandmojo runboth aborted during startup on Windows withAssertion failed: !err.isError(), file Support/lib/Telemetry/TelemetryContext.cpp, line 172and exit code 0xC000001D. The assert is increateLocalIDs, which asks for 32 random bytes to build a session identifier. It got an error every single time.The Windows arm of
SecureRandomBytesGenerator::getRandomBytestestedCryptGenRandomthe wrong way round. It returns aBOOLthat is nonzero on success, and the code returned anErrorwhen it was nonzero. So every successful call was reported as a failure and, worse, every real failure was reported as success with the buffer left untouched, meaning the caller would have got stack garbage and been told it was cryptographically secure.What changed
The condition, plus a comment saying why the reader should not assume the old one was a deliberate inversion of something. The error message now carries
GetLastError(), because a barefalseis all this API gives you and the other two platforms either cannot fail or return the reason themselves.How it was checked
By running the compiler. A cross linked
mojo.exewas staged onto a Windows host in aMODULAR_HOMElayout with the compiler runtime DLL and a Windows built standard library, andmojo buildandmojo runwere pointed at the hello example. Before this change both aborted at the assert above. After it they get past startup.mojo --versionalready worked, which is why this was not caught earlier: the version path does not build a telemetry session.Worth knowing
There is a unit test for this,
Support/unittests/RandomTest.cpp, and it checks precisely the thing that was broken. It has never run on Windows. The tiers intest-tiers.txtcover the Mojo standard library only, and the C++ unit tests are not in any of them. That is the second bug a C++ unit test would have caught, and it is filed separately rather than fixed here.