public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r11-9822] libstdc++: Avoid overflow in bounds checks [PR103955]
@ 2022-04-12 19:18 Patrick Palka
  0 siblings, 0 replies; only message in thread
From: Patrick Palka @ 2022-04-12 19:18 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:0b6d4ee830b01ee70cc5dc32722d73ac3ea4e0db

commit r11-9822-g0b6d4ee830b01ee70cc5dc32722d73ac3ea4e0db
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Jan 12 09:10:24 2022 -0500

    libstdc++: Avoid overflow in bounds checks [PR103955]
    
    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 simplest 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 the approach this patch takes.
    
    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
    fractional 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 __f_t_c_precision.
    
            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 overflow in bounds checks
            similarly.
            * testsuite/20_util/to_chars/103955.cc: New test.
    
    (cherry picked from commit c0e355c77972d96fcec2ff7da047ad03e10e51d9)

Diff:
---
 libstdc++-v3/src/c++17/floating_to_chars.cc       | 46 ++++++++++++++++-------
 libstdc++-v3/testsuite/20_util/to_chars/103955.cc | 31 +++++++++++++++
 2 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc b/libstdc++-v3/src/c++17/floating_to_chars.cc
index 2545022ced6..1116991e555 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_output_end = 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_output_end);
     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..e7f17f7a9e8
--- /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.
+// { dg-do run { target c++17 } }
+
+#include <charconv>
+
+#include <climits>
+#include <initializer_list>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  const int size = 12;
+  char result[size];
+
+  for (auto fmt : { std::chars_format::fixed, std::chars_format::scientific,
+		    std::chars_format::general, std::chars_format::hex })
+    for (int precision : { INT_MAX, INT_MAX-1, INT_MAX-2 })
+      {
+	auto 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();
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-04-12 19:18 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 19:18 [gcc r11-9822] libstdc++: Avoid overflow in bounds checks [PR103955] Patrick Palka

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).