* [PATCH] libstdc++: Avoid overflow in bounds checks [PR103955]
@ 2022-01-11 20:01 Patrick Palka
2022-01-11 21:07 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2022-01-11 20:01 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, Patrick Palka
We currently crash when the floating-point to_chars overloads are passed
a precision value near INT_MAX, ultimately due to overflow in the bounds
checks that verify the output range is large enough.
The most portable fix seems to be to replace bounds checks of the form
A >= B + C (where B + C may overflow) with the otherwise equivalent check
A >= B && A - B >= C, which is what this patch implements.
Before we could do this in __floating_to_chars_hex, there we first need
to track the unbounded "excess" precision (i.e. the number of trailing
digits in the output that are guaranteed to be '0') separately from the
bounded "effective" precision (i.e. the number of significant fractional
digits in the output), like we do in __floating_to_chars_precision.
Tested on x86_64 and i686, does this look OK for trunk/11?
PR libstdc++/103955
libstdc++-v3/ChangeLog:
* src/c++17/floating_to_chars.cc (__floating_to_chars_hex):
Track the excess precision separately from the effective
precision. Avoid overflow in bounds check by splitting it into
two checks.
(__floating_to_chars_precision): Avoid overlflow in bounds
checks similarly.
* testsuite/20_util/to_chars/103955.cc: New test.
---
libstdc++-v3/src/c++17/floating_to_chars.cc | 46 +++++++++++++------
.../testsuite/20_util/to_chars/103955.cc | 31 +++++++++++++
2 files changed, 64 insertions(+), 13 deletions(-)
create mode 100644 libstdc++-v3/testsuite/20_util/to_chars/103955.cc
diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc b/libstdc++-v3/src/c++17/floating_to_chars.cc
index c825a3c8b24..6cd92553d05 100644
--- a/libstdc++-v3/src/c++17/floating_to_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_to_chars.cc
@@ -747,7 +747,8 @@ template<typename T>
__glibcxx_assert(shortest_full_precision >= 0);
int written_exponent = unbiased_exponent;
- const int effective_precision = precision.value_or(shortest_full_precision);
+ int effective_precision = precision.value_or(shortest_full_precision);
+ int excess_precision = 0;
if (effective_precision < shortest_full_precision)
{
// When limiting the precision, we need to determine how to round the
@@ -794,6 +795,11 @@ template<typename T>
}
}
}
+ else
+ {
+ excess_precision = effective_precision - shortest_full_precision;
+ effective_precision = shortest_full_precision;
+ }
// Compute the leading hexit and mask it out from the mantissa.
char leading_hexit;
@@ -816,26 +822,30 @@ template<typename T>
// Now before we start writing the string, determine the total length of
// the output string and perform a single bounds check.
int expected_output_length = sign + 1;
- if (effective_precision != 0)
- expected_output_length += strlen(".") + effective_precision;
+ if (effective_precision + excess_precision > 0)
+ expected_output_length += strlen(".");
+ expected_output_length += effective_precision;
const int abs_written_exponent = abs(written_exponent);
expected_output_length += (abs_written_exponent >= 10000 ? strlen("p+ddddd")
: abs_written_exponent >= 1000 ? strlen("p+dddd")
: abs_written_exponent >= 100 ? strlen("p+ddd")
: abs_written_exponent >= 10 ? strlen("p+dd")
: strlen("p+d"));
- if (last - first < expected_output_length)
+ if (last - first < expected_output_length
+ || last - first - expected_output_length < excess_precision)
return {last, errc::value_too_large};
+ char* const expected_last = first + expected_output_length + excess_precision;
- const auto saved_first = first;
// Write the negative sign and the leading hexit.
if (sign)
*first++ = '-';
*first++ = leading_hexit;
+ if (effective_precision + excess_precision > 0)
+ *first++ = '.';
+
if (effective_precision > 0)
{
- *first++ = '.';
int written_hexits = 0;
// Extract and mask out the leading nibble after the decimal point,
// write its corresponding hexit, and repeat until the mantissa is
@@ -863,13 +873,18 @@ template<typename T>
}
}
+ if (excess_precision > 0)
+ {
+ memset(first, '0', excess_precision);
+ first += excess_precision;
+ }
+
// Finally, write the exponent.
*first++ = 'p';
if (written_exponent >= 0)
*first++ = '+';
const to_chars_result result = to_chars(first, last, written_exponent);
- __glibcxx_assert(result.ec == errc{}
- && result.ptr == saved_first + expected_output_length);
+ __glibcxx_assert(result.ec == errc{} && result.ptr == expected_last);
return result;
}
@@ -1250,7 +1265,8 @@ template<typename T>
}
// Copy the string from the buffer over to the output range.
- if (last - first < output_length + excess_precision)
+ if (last - first < output_length
+ || last - first - output_length < excess_precision)
return {last, errc::value_too_large};
memcpy(first, buffer, output_length);
first += output_length;
@@ -1304,7 +1320,8 @@ template<typename T>
output_length_upper_bound += strlen("e+dd");
int output_length;
- if (last - first >= output_length_upper_bound + excess_precision)
+ if (last - first >= output_length_upper_bound
+ && last - first - output_length_upper_bound >= excess_precision)
{
// The result will definitely fit into the output range, so we can
// write directly into it.
@@ -1325,7 +1342,8 @@ template<typename T>
buffer, nullptr);
__glibcxx_assert(output_length == output_length_upper_bound - 1
|| output_length == output_length_upper_bound);
- if (last - first < output_length + excess_precision)
+ if (last - first < output_length
+ || last - first - output_length < excess_precision)
return {last, errc::value_too_large};
memcpy(first, buffer, output_length);
}
@@ -1365,7 +1383,8 @@ template<typename T>
output_length_upper_bound += strlen(".") + effective_precision;
int output_length;
- if (last - first >= output_length_upper_bound + excess_precision)
+ if (last - first >= output_length_upper_bound
+ && last - first - output_length_upper_bound >= excess_precision)
{
// The result will definitely fit into the output range, so we can
// write directly into it.
@@ -1382,7 +1401,8 @@ template<typename T>
output_length = ryu::d2fixed_buffered_n(value, effective_precision,
buffer);
__glibcxx_assert(output_length <= output_length_upper_bound);
- if (last - first < output_length + excess_precision)
+ if (last - first < output_length
+ || last - first - output_length < excess_precision)
return {last, errc::value_too_large};
memcpy(first, buffer, output_length);
}
diff --git a/libstdc++-v3/testsuite/20_util/to_chars/103955.cc b/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
new file mode 100644
index 00000000000..a47a0a5be8a
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
@@ -0,0 +1,31 @@
+// PR libstdc++/103955
+// Verify we don't crash when the floating-point to_chars overloads are passed
+// a large precision argument.
+
+#include <charconv>
+
+#include <climits>
+#include <initializer_list>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+ constexpr int size = 12;
+ char result[size];
+
+ std::to_chars_result tcr;
+ for (auto fmt : { std::chars_format::fixed, std::chars_format::general,
+ std::chars_format::hex })
+ for (int precision : { INT_MAX, INT_MAX-1 })
+ {
+ tcr = std::to_chars(result, result+size, 1.337, fmt, precision);
+ VERIFY( tcr.ptr == result+size && tcr.ec == std::errc::value_too_large );
+ }
+}
+
+int
+main()
+{
+ test01();
+}
--
2.35.0.rc0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] libstdc++: Avoid overflow in bounds checks [PR103955]
2022-01-11 20:01 [PATCH] libstdc++: Avoid overflow in bounds checks [PR103955] Patrick Palka
@ 2022-01-11 21:07 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2022-01-11 21:07 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc Patches, libstdc++
On Tue, 11 Jan 2022 at 20:03, Patrick Palka via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:
> We currently crash when the floating-point to_chars overloads are passed
> a precision value near INT_MAX, ultimately due to overflow in the bounds
> checks that verify the output range is large enough.
>
> The most portable fix seems to be to replace bounds checks of the form
> A >= B + C (where B + C may overflow) with the otherwise equivalent check
> A >= B && A - B >= C, which is what this patch implements.
>
> Before we could do this in __floating_to_chars_hex, there we first need
> to track the unbounded "excess" precision (i.e. the number of trailing
> digits in the output that are guaranteed to be '0') separately from the
> bounded "effective" precision (i.e. the number of significant fractional
> digits in the output), like we do in __floating_to_chars_precision.
>
> Tested on x86_64 and i686, does this look OK for trunk/11?
>
OK for both, thanks.
> PR libstdc++/103955
>
> libstdc++-v3/ChangeLog:
>
> * src/c++17/floating_to_chars.cc (__floating_to_chars_hex):
> Track the excess precision separately from the effective
> precision. Avoid overflow in bounds check by splitting it into
> two checks.
> (__floating_to_chars_precision): Avoid overlflow in bounds
> checks similarly.
> * testsuite/20_util/to_chars/103955.cc: New test.
> ---
> libstdc++-v3/src/c++17/floating_to_chars.cc | 46 +++++++++++++------
> .../testsuite/20_util/to_chars/103955.cc | 31 +++++++++++++
> 2 files changed, 64 insertions(+), 13 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/to_chars/103955.cc
>
> diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc
> b/libstdc++-v3/src/c++17/floating_to_chars.cc
> index c825a3c8b24..6cd92553d05 100644
> --- a/libstdc++-v3/src/c++17/floating_to_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_to_chars.cc
> @@ -747,7 +747,8 @@ template<typename T>
> __glibcxx_assert(shortest_full_precision >= 0);
>
> int written_exponent = unbiased_exponent;
> - const int effective_precision =
> precision.value_or(shortest_full_precision);
> + int effective_precision = precision.value_or(shortest_full_precision);
> + int excess_precision = 0;
> if (effective_precision < shortest_full_precision)
> {
> // When limiting the precision, we need to determine how to round
> the
> @@ -794,6 +795,11 @@ template<typename T>
> }
> }
> }
> + else
> + {
> + excess_precision = effective_precision - shortest_full_precision;
> + effective_precision = shortest_full_precision;
> + }
>
> // Compute the leading hexit and mask it out from the mantissa.
> char leading_hexit;
> @@ -816,26 +822,30 @@ template<typename T>
> // Now before we start writing the string, determine the total length
> of
> // the output string and perform a single bounds check.
> int expected_output_length = sign + 1;
> - if (effective_precision != 0)
> - expected_output_length += strlen(".") + effective_precision;
> + if (effective_precision + excess_precision > 0)
> + expected_output_length += strlen(".");
> + expected_output_length += effective_precision;
> const int abs_written_exponent = abs(written_exponent);
> expected_output_length += (abs_written_exponent >= 10000 ?
> strlen("p+ddddd")
> : abs_written_exponent >= 1000 ?
> strlen("p+dddd")
> : abs_written_exponent >= 100 ?
> strlen("p+ddd")
> : abs_written_exponent >= 10 ?
> strlen("p+dd")
> : strlen("p+d"));
> - if (last - first < expected_output_length)
> + if (last - first < expected_output_length
> + || last - first - expected_output_length < excess_precision)
> return {last, errc::value_too_large};
> + char* const expected_last = first + expected_output_length +
> excess_precision;
>
> - const auto saved_first = first;
> // Write the negative sign and the leading hexit.
> if (sign)
> *first++ = '-';
> *first++ = leading_hexit;
>
> + if (effective_precision + excess_precision > 0)
> + *first++ = '.';
> +
> if (effective_precision > 0)
> {
> - *first++ = '.';
> int written_hexits = 0;
> // Extract and mask out the leading nibble after the decimal point,
> // write its corresponding hexit, and repeat until the mantissa is
> @@ -863,13 +873,18 @@ template<typename T>
> }
> }
>
> + if (excess_precision > 0)
> + {
> + memset(first, '0', excess_precision);
> + first += excess_precision;
> + }
> +
> // Finally, write the exponent.
> *first++ = 'p';
> if (written_exponent >= 0)
> *first++ = '+';
> const to_chars_result result = to_chars(first, last,
> written_exponent);
> - __glibcxx_assert(result.ec == errc{}
> - && result.ptr == saved_first +
> expected_output_length);
> + __glibcxx_assert(result.ec == errc{} && result.ptr == expected_last);
> return result;
> }
>
> @@ -1250,7 +1265,8 @@ template<typename T>
> }
>
> // Copy the string from the buffer over to the output range.
> - if (last - first < output_length + excess_precision)
> + if (last - first < output_length
> + || last - first - output_length < excess_precision)
> return {last, errc::value_too_large};
> memcpy(first, buffer, output_length);
> first += output_length;
> @@ -1304,7 +1320,8 @@ template<typename T>
> output_length_upper_bound += strlen("e+dd");
>
> int output_length;
> - if (last - first >= output_length_upper_bound + excess_precision)
> + if (last - first >= output_length_upper_bound
> + && last - first - output_length_upper_bound >=
> excess_precision)
> {
> // The result will definitely fit into the output range, so we
> can
> // write directly into it.
> @@ -1325,7 +1342,8 @@ template<typename T>
> buffer, nullptr);
> __glibcxx_assert(output_length == output_length_upper_bound - 1
> || output_length ==
> output_length_upper_bound);
> - if (last - first < output_length + excess_precision)
> + if (last - first < output_length
> + || last - first - output_length < excess_precision)
> return {last, errc::value_too_large};
> memcpy(first, buffer, output_length);
> }
> @@ -1365,7 +1383,8 @@ template<typename T>
> output_length_upper_bound += strlen(".") + effective_precision;
>
> int output_length;
> - if (last - first >= output_length_upper_bound + excess_precision)
> + if (last - first >= output_length_upper_bound
> + && last - first - output_length_upper_bound >=
> excess_precision)
> {
> // The result will definitely fit into the output range, so we
> can
> // write directly into it.
> @@ -1382,7 +1401,8 @@ template<typename T>
> output_length = ryu::d2fixed_buffered_n(value,
> effective_precision,
> buffer);
> __glibcxx_assert(output_length <= output_length_upper_bound);
> - if (last - first < output_length + excess_precision)
> + if (last - first < output_length
> + || last - first - output_length < excess_precision)
> return {last, errc::value_too_large};
> memcpy(first, buffer, output_length);
> }
> diff --git a/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
> b/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
> new file mode 100644
> index 00000000000..a47a0a5be8a
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
> @@ -0,0 +1,31 @@
> +// PR libstdc++/103955
> +// Verify we don't crash when the floating-point to_chars overloads are
> passed
> +// a large precision argument.
> +
> +#include <charconv>
> +
> +#include <climits>
> +#include <initializer_list>
> +#include <testsuite_hooks.h>
> +
> +void
> +test01()
> +{
> + constexpr int size = 12;
> + char result[size];
> +
> + std::to_chars_result tcr;
> + for (auto fmt : { std::chars_format::fixed, std::chars_format::general,
> + std::chars_format::hex })
> + for (int precision : { INT_MAX, INT_MAX-1 })
> + {
> + tcr = std::to_chars(result, result+size, 1.337, fmt, precision);
> + VERIFY( tcr.ptr == result+size && tcr.ec ==
> std::errc::value_too_large );
> + }
> +}
> +
> +int
> +main()
> +{
> + test01();
> +}
> --
> 2.35.0.rc0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-11 21:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 20:01 [PATCH] libstdc++: Avoid overflow in bounds checks [PR103955] Patrick Palka
2022-01-11 21:07 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).