Skip to content

<format>: fix single byte out of bounds access - #2569

Merged
Stephan T. Lavavej (StephanTLavavej) merged 1 commit into
microsoft:mainfrom
barcharcraz:fix_format_bogus_read
Feb 18, 2022
Merged

Stephan T. Lavavej (StephanTLavavej) merged 1 commit into
microsoft:mainfrom
barcharcraz:fix_format_bogus_read

Conversation

@barcharcraz

@barcharcraz Charlie Barto (barcharcraz) commented Feb 15, 2022

Copy link
Copy Markdown
Contributor

format would access uninitialized stack when trying to find the position of the exponent. When the junk in _Buffer (or past it, if to_chars used up all available space in the buffer) happened to be 'p' in hex mode (or 'e' in scientific mode, or their uppercased forms), this produced incorrect results.

Fixes #2449. Thanks to statementreply for investigating!

When the junk in _Buffer happened to be 'p' (or if to_chars used up
all available space in the buffer, then format would access
uninitialized stack when trying to find the position of the "e"
in scientific mode.
@barcharcraz Charlie Barto (barcharcraz) added the format C++20/23 format label Feb 15, 2022
@barcharcraz
Charlie Barto (barcharcraz) requested a review from a team as a code owner February 15, 2022 03:06
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added the bug Something isn't working label Feb 15, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

As mentioned on Discord, I believe that this fix is minimal and safe. We know that to_chars() has succeeded, the value is finite, and we're in hex or scientific mode, so it has surely written an exponent character. Thus, the "walk backwards" logic can always step backwards before reading the character.

Charlie Barto (@barcharcraz) would like to leave #2494's additional diagnostics around for a little while after merging this, which sounds good to me (I've added it to my list of cleanups for later).

FYI Casey Carter (@CaseyCarter), I believe this is a must-have to backport to 16.11.x with the rest of the <format> changes.

Comment thread stl/inc/format
@CaseyCarter Casey Carter (CaseyCarter) changed the title <format>: fix single byte out of bounds access <format>: fix single byte out of bounds access Feb 15, 2022
@CaseyCarter

Copy link
Copy Markdown
Contributor

FYI Casey Carter (@CaseyCarter), I believe this is a must-have to backport to 16.11.x with the rest of the <format> changes.

Added to my informal list of stuff to backport.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urgh, sorry for missing that when reworking the additional zeros path

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. (I'll handle verifying that this works with the recent LLVM update.)

Stephan T. Lavavej (StephanTLavavej) added a commit to StephanTLavavej/STL that referenced this pull request Feb 17, 2022
@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 113cb97 into microsoft:main Feb 18, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for fixing the root cause of the mysterious sporadic failures! 🎉 ✅ 😺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working format C++20/23 format

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sporadic failure of P0645R10_text_formatting_formatting

4 participants