Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing large integer value invokes undefined behaviour #188

Closed
ghost opened this issue Jan 22, 2023 · 5 comments
Closed

Parsing large integer value invokes undefined behaviour #188

ghost opened this issue Jan 22, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@ghost
Copy link

ghost commented Jan 22, 2023

Environment

toml++ version and/or commit hash:
version 3.2.0, commit 698285d on master branch

Compiler:
GCC 10.2.1

C++ standard mode:
-std=c++17

Target arch:
x86_64

Library configuration overrides:
none

Relevant compilation flags:
-std=c++17 -O2 -fsanitize=undefined -fsanitize=address

Describe the bug

When parsing the integer value -9223372036854775808 (i.e. -2^63), the parser constructs an intermediate uint64_t value 9223372036854775808.
It then casts this value to int64_t, which has implementation defined behaviour and results in -9223372036854775808.
It then attempts to multiply this value by -1 which has undefined behaviour because the result can not be represented as int64_t.

Steps to reproduce (or a small repro code sample)

Compile as follows:
g++ -Iinclude -Wall -Wextra -Wpedantic -Werror -std=c++17 -O2 -fsanitize=undefined -fsanitize=address -o tt_decoder toml-test/tt_decoder.cpp

Then run tt_decoder on the following TOML document:

v = -9223372036854775808

The program prints the following error message:

include/toml++/impl/parser.inl:2272:44: runtime error: signed integer overflow: -1 * -9223372036854775808 cannot be represented in type 'long int'

Additional information

@ghost ghost added the bug Something isn't working label Jan 22, 2023
@ghost ghost assigned marzer Jan 22, 2023
@marzer
Copy link
Owner

marzer commented Jan 22, 2023

Nice find! Will look into this later today :)

@marzer
Copy link
Owner

marzer commented Jan 22, 2023

Sorry, ran out of time. Will have a look during the week.

@marzer
Copy link
Owner

marzer commented Jan 29, 2023

Looking into this now. There's already checks where I'm rejecting numbers that are unrepresentable, just that there's a gap in the sign handling logic for exactly INT64_MIN 😅

@ghost
Copy link
Author

ghost commented Jan 29, 2023

You're right 😄
I'm almost sorry for reporting it. But it's still worthwhile to eliminate an undefined codepath I guess.

@marzer
Copy link
Owner

marzer commented Jan 29, 2023

Eh, I'm happy you did. It's probably not that uncommon a value in the wild since people often use INT_MIN/MAX as sentinels for various things.

@marzer marzer closed this as completed in 8f31ec8 Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant