RFC: Adopt C++ Safe Buffers? #31272

issue maflcko openend this issue on November 11, 2024
  1. maflcko commented at 2:30 pm on November 11, 2024: member

    C++ is unsafe, meaning that any code written in it may cause undefined behavior at runtime. One possible source of undefined behavior is out-of-range memory access.

    While some limited compiler warnings exist to detect some obvious cases, tracking down out-of-range memory access is usually done at runtime with debugging tools such as Valgrind or Asan. However, such tools can normally not be used in production, because they are not hardening tools, see https://stackoverflow.com/a/70004411/2084795. Some C++ standard libraries provide options to enable a hardened build, which can also be used in production, see https://libcxx.llvm.org/Hardening.html.

    However, this requires using the standard library containers or primitives to represent buffers. For example, instead of using a raw C-array, std::array should be preferred. Also, instead of using a raw C-pointer, std::span should be preferred.

    My understanding is that only libc++ offers such a hardened build right now, so the benefit would be limited. Also, the required patch is large-ish. However, I think it would be good to keep this hardening feature in mind and use std::array and std::span for new code. Possibly in the future, those can be enforced. std::array via https://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-c-arrays.html and std::span (really all buffer representations) via -Wunsafe-buffer-usage https://clang.llvm.org/docs/SafeBuffers.html.

  2. maflcko added the label Brainstorming on Nov 11, 2024
  3. laanwj commented at 11:02 am on November 14, 2024: member

    However, this requires using the standard library containers or primitives to represent buffers. For example, instead of using a raw C-array, std::array should be preferred. Also, instead of using a raw C-pointer, std::span should be preferred.

    i think we’ve already been going in this direction for quite a while.

  4. dergoegge commented at 7:52 pm on November 15, 2024: member

    My understanding is that only libc++ offers such a hardened build right now

    -D_GLIBCXX_ASSERTIONS appears to enable the same (or similar) for libstdc++, see here.

  5. vasild commented at 12:09 pm on November 21, 2024: contributor

    Concept ACK

    required patch is large-ish

    Which patch exactly? To switch everything to use std::array and make clang-tidy-avoid-c-arrays happy (and similar for std::span)? Approach ACK on that one ;)

  6. ldionne commented at 10:37 pm on December 4, 2024: none

    Libc++ maintainer here. I don’t know how much existing use of the C++ Standard Library you have in the code base today, however one thing you could do is enable hardening today and then your code will get safer the more standard library usage you introduce. Adoption-wise, that can be easier and it can also provide a nice incentive for folks to use the standard library instead of e.g. C arrays and raw pointers. In other words, you don’t need to already be using the standard library extensively to enable hardening.

    libc++: -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_{FAST,EXTENSIVE,DEBUG} (for production, we recommend _LIBCPP_HARDENING_MODE_FAST libstdc++: -D_GLIBCXX_ASSERTIONS which should be more or less equivalent to libc++’s fast mode

    If you do enable hardening and have questions or run into issues (with libc++), feel free to @ldionne or @var-const and we can probably help out.

  7. vasild referenced this in commit 0398eb63a3 on Dec 5, 2024
  8. vasild commented at 6:22 am on December 5, 2024: contributor
    @ldionne, thanks for the suggestion! Followed up in #31424.

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me