libstdc++: Fix std::from_chars to not read past end of input [PR127666]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
The bug report points out one out-of-bounds read and the second was
found by an LLM. The testcase was generated by an LLM. The code fixes
are artisanal handwritten code.
Assisted-by: LLM
libstdc++-v3/ChangeLog:
PR libstdc++/127666
* src/c++17/floating_from_chars.cc (pattern): Avoid reading past
the end of the input. Remove unused variable.
* testsuite/20_util/from_chars/127666.cc: New test.
---
Tested x86_64-linux.
I'm not sure whether the mmap and sysconf effective targets are enough
to ensure the new test works everywhere. If we get new FAILs for it we
can adjust the test to not run on more targets.
libstdc++-v3/src/c++17/floating_from_chars.cc | 9 ++-
.../testsuite/20_util/from_chars/127666.cc | 70 +++++++++++++++++++
2 files changed, 76 insertions(+), 3 deletions(-)
create mode 100644 libstdc++-v3/testsuite/20_util/from_chars/127666.cc
Comments
On Thu, Oct 1, 2026 at 7:46 PM Jonathan Wakely <jwakely@redhat.com> wrote:
> The bug report points out one out-of-bounds read and the second was
> found by an LLM. The testcase was generated by an LLM. The code fixes
> are artisanal handwritten code.
>
> Assisted-by: LLM
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/127666
> * src/c++17/floating_from_chars.cc (pattern): Avoid reading past
> the end of the input. Remove unused variable.
> * testsuite/20_util/from_chars/127666.cc: New test.
> ---
>
> Tested x86_64-linux.
>
The assign(first, 0, len) is a really sneaky bug. LTGM.
>
> I'm not sure whether the mmap and sysconf effective targets are enough
> to ensure the new test works everywhere. If we get new FAILs for it we
> can adjust the test to not run on more targets.
>
> libstdc++-v3/src/c++17/floating_from_chars.cc | 9 ++-
> .../testsuite/20_util/from_chars/127666.cc | 70 +++++++++++++++++++
> 2 files changed, 76 insertions(+), 3 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/from_chars/127666.cc
>
> diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc
> b/libstdc++-v3/src/c++17/floating_from_chars.cc
> index ed434b395288..c28fc9a459ef 100644
> --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> @@ -451,13 +451,16 @@ namespace
> // fmt has the value of one of the enumerators of chars_format.
> __glibcxx_assert(valid_fmt(fmt));
>
> - string_view res;
> -
> + // Fail if the input is empty or starts with '+'
> if (first == last || *first == '+') [[unlikely]]
> return nullptr;
>
> const int neg = (*first == '-');
>
> + // Fail if the input consists of a lone '-'
> + if (first + neg == last)
> + return nullptr;
> +
> if (std::memchr("iInN", (unsigned char)first[neg], 4))
> {
> ptrdiff_t len = last - first;
> @@ -489,7 +492,7 @@ namespace
> else // Only need 4 chars for "-NAN"
> len = neg + 3;
>
> - buf.assign(first, 0, len);
> + buf.assign(first, len);
> // prevent make_result correcting for "0x"
> fmt = chars_format::general;
> return buf.c_str();
> diff --git a/libstdc++-v3/testsuite/20_util/from_chars/127666.cc
> b/libstdc++-v3/testsuite/20_util/from_chars/127666.cc
> new file mode 100644
> index 000000000000..25989f2cfee8
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/from_chars/127666.cc
> @@ -0,0 +1,70 @@
> +// { dg-do run { target c++17 } }
> +// { dg-require-effective-target mmap }
> +// { dg-require-effective-target sysconf }
> +// { dg-add-options ieee }
> +// { dg-additional-options "-DSKIP_LONG_DOUBLE" { target aarch64-*-rtems*
> aarch64-*-vxworks* x86_64-*-vxworks* } }
> +
> +// Bug 127666
> +// std::from_chars(first, last, long double&) reads past 'last' for
> "inf"/"nan"
> +
> +#include <charconv>
> +#include <string_view>
> +#include <cstring>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +#include <testsuite_hooks.h>
> +
> +using namespace std::string_view_literals;
> +
> +// One past the last writable byte; the guard page starts here, so reading
> +// at or beyond this address faults.
> +static char* buf_end;
> +
> +template<typename T>
> +void
> +check(std::string_view input, std::errc ec, std::size_t len)
> +{
> + // Position the input against the guard page so an over-read faults.
> + char* const first = buf_end - input.size();
> + std::memcpy(first, input.data(), input.size());
> + T value = 1;
> + auto res = std::from_chars(first, buf_end, value);
> + VERIFY( res.ec == ec );
> + VERIFY( res.ptr == first + len );
> +}
> +
> +template<typename T>
> +void
> +test_type()
> +{
> + // The inf/nan forms used to read past the end via strlen.
> + check<T>("inf"sv, std::errc{}, 3);
> + check<T>("infinity"sv, std::errc{}, 8);
> + check<T>("nan"sv, std::errc{}, 3);
> + check<T>("nan(1)"sv, std::errc{}, 6);
> + check<T>("-nan(ab)"sv, std::errc{}, 8);
> +
> + // A lone sign used to read one byte past the end.
> + check<T>("-"sv, std::errc::invalid_argument, 0);
> + check<T>("+"sv, std::errc::invalid_argument, 0);
> +}
> +
> +int
> +main()
> +{
> + const long pg = ::sysconf(_SC_PAGESIZE);
> + VERIFY( pg > 0 );
> + char* const base = (char*) ::mmap(nullptr, 2 * pg, PROT_READ |
> PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + VERIFY( base != MAP_FAILED );
> + VERIFY( ::mprotect(base + pg, pg, PROT_NONE) == 0 );
> + buf_end = base + pg;
> +
> + test_type<float>();
> + test_type<double>();
> +#ifndef SKIP_LONG_DOUBLE
> + test_type<long double>();
> +#endif
> +
> + ::munmap(base, 2 * pg);
> +}
> --
> 2.55.0
>
>
@@ -451,13 +451,16 @@ namespace
// fmt has the value of one of the enumerators of chars_format.
__glibcxx_assert(valid_fmt(fmt));
- string_view res;
-
+ // Fail if the input is empty or starts with '+'
if (first == last || *first == '+') [[unlikely]]
return nullptr;
const int neg = (*first == '-');
+ // Fail if the input consists of a lone '-'
+ if (first + neg == last)
+ return nullptr;
+
if (std::memchr("iInN", (unsigned char)first[neg], 4))
{
ptrdiff_t len = last - first;
@@ -489,7 +492,7 @@ namespace
else // Only need 4 chars for "-NAN"
len = neg + 3;
- buf.assign(first, 0, len);
+ buf.assign(first, len);
// prevent make_result correcting for "0x"
fmt = chars_format::general;
return buf.c_str();
new file mode 100644
@@ -0,0 +1,70 @@
+// { dg-do run { target c++17 } }
+// { dg-require-effective-target mmap }
+// { dg-require-effective-target sysconf }
+// { dg-add-options ieee }
+// { dg-additional-options "-DSKIP_LONG_DOUBLE" { target aarch64-*-rtems* aarch64-*-vxworks* x86_64-*-vxworks* } }
+
+// Bug 127666
+// std::from_chars(first, last, long double&) reads past 'last' for "inf"/"nan"
+
+#include <charconv>
+#include <string_view>
+#include <cstring>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <testsuite_hooks.h>
+
+using namespace std::string_view_literals;
+
+// One past the last writable byte; the guard page starts here, so reading
+// at or beyond this address faults.
+static char* buf_end;
+
+template<typename T>
+void
+check(std::string_view input, std::errc ec, std::size_t len)
+{
+ // Position the input against the guard page so an over-read faults.
+ char* const first = buf_end - input.size();
+ std::memcpy(first, input.data(), input.size());
+ T value = 1;
+ auto res = std::from_chars(first, buf_end, value);
+ VERIFY( res.ec == ec );
+ VERIFY( res.ptr == first + len );
+}
+
+template<typename T>
+void
+test_type()
+{
+ // The inf/nan forms used to read past the end via strlen.
+ check<T>("inf"sv, std::errc{}, 3);
+ check<T>("infinity"sv, std::errc{}, 8);
+ check<T>("nan"sv, std::errc{}, 3);
+ check<T>("nan(1)"sv, std::errc{}, 6);
+ check<T>("-nan(ab)"sv, std::errc{}, 8);
+
+ // A lone sign used to read one byte past the end.
+ check<T>("-"sv, std::errc::invalid_argument, 0);
+ check<T>("+"sv, std::errc::invalid_argument, 0);
+}
+
+int
+main()
+{
+ const long pg = ::sysconf(_SC_PAGESIZE);
+ VERIFY( pg > 0 );
+ char* const base = (char*) ::mmap(nullptr, 2 * pg, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ VERIFY( base != MAP_FAILED );
+ VERIFY( ::mprotect(base + pg, pg, PROT_NONE) == 0 );
+ buf_end = base + pg;
+
+ test_type<float>();
+ test_type<double>();
+#ifndef SKIP_LONG_DOUBLE
+ test_type<long double>();
+#endif
+
+ ::munmap(base, 2 * pg);
+}