[Bug] Protential stack-buffer-overflow crash in fptostring.cpp #1385
Description
Hi team! Thanks for your great work!
I think I found a small vulnerability that might lead to crash in the system.
It locates at the std::string FpToString() function line 187 in fptostring.cpp. The vulnerability occurs because a high precision parameter allows a large number to be formatted without scientific notation, causing a fixed 28-byte stack buffer to be overflowed.
PoC
We can add the following code to test/fptostring_test.cpp
TEST(FpToStringTest, vulnerability_stack_buffer_overflow) {
FpToString(1.0e100, 200);
}and run ../build/test/yaml-cpp-tests --gtest_filter=FpToStringTest.vulnerability_stack_buffer_overflow
We can see
==279788==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffd21c at pc 0x555555fa31b0 bp 0x7fffffffcef0 sp 0x7fffffffcee0
WRITE of size 1 at 0x7fffffffd21c thread T0
#0 0x555555fa31af in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > YAML::detail::fp_formatting::FpToString<double>(double, int) /yaml-cpp_master/src/fptostring.cpp:187
#1 0x555555f9ee5b in YAML::FpToString[abi:cxx11](double, unsigned long) /yaml-cpp_master/src/fptostring.cpp:221Root Causes
The crash is caused by that, when the precision of parameter get very large, say 1.0e100, it will lead to the loop run for 100 times, leading to overflow at output_ptr++.
std::array<char, 28> output_buffer;
...
int const before_decimal_digits = digits_ct + r.exponent; // r.exponent = 100
...
for (int i{0}; i < before_decimal_digits - digits_ct; ++i) { // loop executes ~100 iterations,
*(output_ptr++) = '0'; // crash as output_buffer only has 28 bytes
}Fix
I've checked the whole file and I think add a check before writing to the buffer at line 187, or simply replace the fixed array with a dynamically sized buffer would be enough. I tried on my own machine and the crash was successfully fixed.