Skip to content

Commit c7aa78d

Browse files
SGSSGenejbeder
authored andcommitted
fix: overflow buffer with large precision values
Issue #1385 demonstrates how a large 'precision' value can cause buffer overflows. Originally, the buffer was designed to fit any scientific notation. But the precision changes at which point large floating point numbers are displayed in scientific notation or default notation. In case of the default notation many extra zeros have to be printed, this was not reflected in the output_buffer and an overflow could occur. This PR computes the number of zero that do not fit into the static buffer and appends them at the end of the function triggering potential a second dynamic allocation. (The first allocation is the std::string allocation).
1 parent a2826e8 commit c7aa78d

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

src/fptostring.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,14 @@ std::string FpToString(T v, int precision = 0) {
129129
}
130130
}
131131

132-
std::array<char, 28> output_buffer; // max digits of size_t plus sign, a dot and 2 letters for 'e+' or 'e-' and 4 letters for the exponent
132+
// Case 1 - scientific notation: max digits of size_t plus sign, a dot and 2 letters for 'e+' or 'e-' and 4 letters for the exponent
133+
// Case 2 - default notation: require up to precision number of digits and one for a potential sign
134+
std::array<char, 28> output_buffer;
133135
auto output_ptr = &output_buffer[0];
134136

137+
// Helper variable that in Case 2 counts the overflowing number of zeros that do not fit into the buffer.
138+
int overflow_zeros = 0;
139+
135140
// print '-' symbol for negative numbers
136141
if (r.is_negative) {
137142
*(output_ptr++) = '-';
@@ -172,18 +177,28 @@ std::string FpToString(T v, int precision = 0) {
172177

173178
// case 2: default notation
174179
} else {
175-
auto const digits_end = digits.begin() + digits_ct;
176-
auto digits_iter = digits.begin();
180+
auto const digits_end = digits.begin() + digits_ct;
181+
auto digits_iter = digits.begin();
177182

178183
// print digits before point
179184
int const before_decimal_digits = digits_ct + r.exponent;
180185
if (before_decimal_digits > 0) {
181-
// print digits before point
186+
// print non-zero digits before point
182187
for (int i{0}; i < std::min(before_decimal_digits, digits_ct); ++i) {
183188
*(output_ptr++) = *(digits_iter++);
184189
}
190+
191+
// number of digits that have to be zero
192+
int const zero_digits_ct = before_decimal_digits - digits_ct;
193+
194+
// space left in the output_buffer (-1 because we need it for null-termination)
195+
int const buffer_empty_space = output_buffer.data() + output_buffer.size() - output_ptr - 1;
196+
197+
// print all zeros not fitting into the buffer at the end of the function
198+
overflow_zeros = std::max(0, zero_digits_ct - buffer_empty_space);
199+
185200
// print trailing zeros before point
186-
for (int i{0}; i < before_decimal_digits - digits_ct; ++i) {
201+
for (int i{0}; i < zero_digits_ct - overflow_zeros; ++i) {
187202
*(output_ptr++) = '0';
188203
}
189204

@@ -207,7 +222,9 @@ std::string FpToString(T v, int precision = 0) {
207222
}
208223
}
209224
*output_ptr = '\0';
210-
return std::string{&output_buffer[0], output_ptr};
225+
auto ret_value = std::string{&output_buffer[0], output_ptr};
226+
ret_value.resize(ret_value.size() + overflow_zeros, '0');
227+
return ret_value;
211228
}
212229

213230
}

test/fptostring_test.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,5 +239,9 @@ TEST(FpToStringTest, conversion_float) {
239239
EXPECT_EQ("-1.3e-05", FpToString(-1.299e-5f, 2));
240240
}
241241

242+
TEST(FpToStringTest, vulnerability_stack_buffer_overflow) {
243+
EXPECT_EQ(FpToString(1.0e100, 200), "10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000");
244+
}
245+
242246
} // namespace
243247
} // namespace YAML

0 commit comments

Comments
 (0)