10 ms·

RFC: C++ Buffer Hardening

112 points by gok 4y ago

sposeray 4y ago
[dead]
ardel95 4y ago
I’m afraid this might be too little, too late. But regardless whether I’m right or wrong, we all should welcome a widely used language trying to become a little more safe.
- yazzku 4y ago
  On the other hand, it appears to have a great bang/buck. Link existing programs against the hardened libc++ and get those checks for "free" (other than the perf hit.)
  - jeffbee 4y ago
    I'm not at all clear on how exactly that would work. The entire program would have to be rebuilt from scratch, would it not? Things like std::vector::operator[] are not runtime library calls, they are almost always inlined by the compiler. You could not simply take an existing program, replace its runtime library, and get bounds-checked vector accesses.
    - pjmlp 4y ago
      Depends how the linking took place, and optimization levels.
      - planede 4y ago
        vector is a template, at most vector<some_type>::operator[] will be a weak symbol, and I bet it's inlined from -O1 and above.
        gpderetta 4y ago
        Even if not inlined at -O0, when instantiated for a custom some_type, the standard library .so won't contain any code for it, everything will be in the application binary.
        pjmlp 4y ago
        Depends on the implementation of helper functions and if external templates are being used.
        gpderetta 4y ago
        extern templates won't help for user defined types of course as the standard library couldn't possibly have instantiations for them :). Some non template helpers might be improved of course.
        pjmlp 4y ago
        They help for common types like std::string.
        pjmlp 4y ago
        On Windows it can still live on a dynamic library. That has been a thing since the Windows 16 bit days, and other OSes with similar linking models like Symbian, OS/2 and AIX.
        jeffbee 4y ago
        It’s difficult to imagine which applications would find the performance of an outline vector access acceptable. Generally vector::operator[] is represented in built programs by a single x86 mov. A call would wreck the performance. In languages with bounds-checked vectors, they don’t do it with library calls.
        pjmlp 4y ago
    - gpderetta 4y ago
      Of course, it is a compiler feature, you will have to recompile and manually (or automatically) apply all suggested changes to the source code. If you just link against an hardened library, only the implementation of the (non inlined) library calls will be hardened, the user code itself won't see much benefits. Of course these days we have binary level optimizers, so who knows what it is possible in principle?
    - yazzku 4y ago
      Sorry, I used 'link' very liberally there, but you cleared it up. My point was that (from what I remember, it's been a few days) you'd mostly just need to recompile. As opposed to, you know, Rewrite It In Rust (TM).
- green_on_black 4y ago
  I agree. But the butthurt side of me wants to say: let those who wish for unsafety be unsafe.
  - atoav 4y ago
    But programming is multiplication. A programmers work multiplies. If some person once writes code that ends up leaking users data, that data might like a thousand users data in a way that cannot be unleaked. That broken code then multiplies through time. You would not say: Let those [civil engineers] who like unsafe bridges build unsafe bridges Because the people using them might not be able to judge the unsafety of the system. Because the work of programmers multiply we have a huge responsibility and a huge chance to improve the world.
    - rapidlua 4y ago
      But c’mon there’s no such thing as safe brain surgery. Would you then market brain surgeries as unsafe?
      - rurban 4y ago
        Actually working on safe brain surgery robots. It's marketed as safe, and the code is checked to be hard-realtime and safe.
      - yakubin 4y ago
        Yes? I wouldn’t even think that’s in any way controversial.
      - samus 4y ago
        Bad comparison. Brain surgeries are a method of last resort. They are unsafe because there are significant knowledge gaps in our understanding of the brain that might take centuries more to fill. Every time a neurosurgeon cuts, they are potentially doing irreversible damage to an important part of a person's consciousness, abilities and memories. Edit: and there is no way to prevent that damage except by detailed planning and by stopping to cut when the patient (under local anestesia) ceases talking or performing their craft. Other damage can only be detected later though.
- pjmlp 4y ago
  Given that in many domains C++ will keep being the language to use, and most wannabe replacements depend on GCC and LLVM infrastructure (written in C++), any little bit of improved safety helps.
- rychco 4y ago
  Too late for what? C++ will continue to be used for decades.
  - scrubs 4y ago
    If C++ could, and I think with some respect and acknowledgement to others as having gotten it better, adopt two improvements: * library/language side: some domain of bullet proof memory * compiler/language side: move build dependencies into the compiler or something that frontends the compiler ala GO/Rust with a flag to enforce it so it's optional. Here don't let perfect be the enemy of the good. There'd need be some new keywords or namespaces to make this work. That'd be superb I have all kinds of chances and opportunities to learn Rust but our eco system is endless C/C++. So I'd rather not deal with another complicated language. There's no near term or medium term timeline to move even 20% into Rust. I bail out to GO for I/O bound work.
    - maldev 4y ago
      There's multiple frameworks which give bullet proof memory usages and other fixes through annotations. The most common being https://learn.microsoft.com/en-us/cpp/code-quality/understanding-sal?view=msvc-170 https://learn.microsoft.com/en-us/cpp/code-quality/understan... . People just dont use them besides kernel devs, and even then they don't really use it. I believe the entire MS kernel is written using this.
      - pianoben 4y ago
        a) Not just the Windows kernel - Office uses SAL, too, in numerous places. Mostly in legacy places predating more modern C++. b) It's not true to say that kernel devs don't really use it. They use it a lot, though there are attempts at obsoleting some of it[0]. [0]: https://github.com/microsoft/wil https://github.com/microsoft/wil
        maldev 4y ago
        Oh nice, I didn't know that about office, it makes sense though. I don't work for Microsoft and mainly work in the Antivirus space, so I only occasionally hear stories from friends about their internal stuff. When I said driver devs don't always use it, I mainly meant third party vendors writing kernel code. I know alot of the overseas places don't, and I've rarely had jobs which use it.
deleted 4y ago
[deleted]
yazzku 4y ago
saagarjha 4y ago
> For example, accessing a std::span or a std::vector outside of its bounds would abort the program, and so would accessing an empty std::optional. This can be done while staying Standards-conforming because undefined behavior implies that the library can do whatever it wants, which includes aborting. sickosyes.jpg Getting bad operations to abort execution (e.g. by throwing an exception) seems impossible to ship, not in the least due to certain, uh, entities really being against it. But abusing UB to allow people to turn on checks is excellent. This is why we should be very careful about what we choose to standardize-a common request is making signed overflow wrap instead of being UB, which prevents its diagnosis in this way.
- olliej 4y ago
  Making out of bounds access on explicitly bound types is an easy memory safety win. The fact the UB is still being used as an excuse to clearly undermine reasonably expected behaviour remains a BS trope, and WG21's continued acceptance of new features and specifications that say perfectly specifiable behaviour is UB instead of unspecified is a continuing source of avoidable security bugs. It is inexcusable for new features to be added to C++ that have UB in any case other than the undefinable (invalid memory access, etc). Removing the copious unnecessary UB from the existing specs should be a higher priority that many of the new features being proposed.
  - saagarjha 4y ago
    Much that is UB deserves to be defined. And some that is defined deserves to be UB. Can you make it illegal? Then do not be too eager to deal out behaviors in standardization. I don't actually oppose, in the long run, slowly picking some behaviors and changing them to be standardized. But I do vehemently oppose many of the suggestions that are brought up. The kinds of things that should be moved out of being undefined are things like "you named your function isfoo" and not "you passed NULL to memset".
    - olliej 4y ago
      > And some that is defined deserves to be UB. Can you provide an example of currently specified behaviour that should be UB? To be very clear I am using specification language here. "Unspecified behaviour" is very much not "undefined behaviour".
rapidlua 4y ago
This is unfortunately ABI-altering, therefore it won’t be possible to selectively enable for projects that want it. It’s a distribution that has either to bite the bullet and say “we are going to pessimize all C++ software”, or err on the safe side and leave it disabled. The warning about pointer arithmetic is most likely suppressable per compilation unit (for things like tagged pointers).
- olliej 4y ago
  What is the ABI altering concern? Switching from pointer+length->span? Because yeah that is alas a source of suffering. > “we are going to pessimize all C++ software” Could you clarify?
  - rapidlua 4y ago
    As of today, vector’s iterator is just a pointer. You need more than just a pointer to detect out of bounds accesses. It makes old binaries incompatible with new binaries. It also means that you can’t mix and match, it must be either a or b. Hence you can’t make the change incrementally, which is a big risk. Concerning Linux distributions, I seriously doubt if they have resources to assess the performance impact across the wide range of software. Concerning performance impact, we end up with more instructions and more memory accesses and increased register pressure. This has a chance to make things slower.
    - pabs3 4y ago
      Perhaps you could keep ABI compatible using with versioned symbols?
      - gpderetta 4y ago
        If the functions are separate overloads there is no need of versioning in the first place. On the other hand if you change the layout of structures (like it would be the case for bound checked pointers), it is much much harder. GCC has ABI tags, but they only help a bit. In any case nobody is going to bound check iterators by default, these already exists when using whatever debug standard library your compiler provides and they are orders of magnitude slower than unchecked iterators.
gpderetta 4y ago
If I'm reading this correctly, this has two components: - a bound checked libc++, which is not particularly exciting, libstdc++ has had _GLIBCXX_ASSERTIONS for a while for example. In fact it seems that libc++ has had it as well. - clang warnings to flag potentially unsafe code plus fix-it suggestions to convert it to safer code using the bound checked libc++. The later is quite interesting, especially the fix-it might make it easier to incrementally "harden" a large c++ code base.
- rwmj 4y ago
  The first one also sounds a bit like -D_FORTIFY_SOURCE=2 in glibc which fortifies memcpy etc. Many Linux distros have had this enabled by default for years. (https://stackoverflow.com/a/16604146 https://stackoverflow.com/a/16604146) Agree the second one both sounds interesting and also quite disruptive to existing code.
  - fweimer 4y ago
    _GLIBCXX_ASSERTIONS is slightly different because it uses the bounds information that is already available. The historical expectation is that operator[] doesn't perform bounds checking by default, you need to use the at(size_type) members for that. Hence _GLIBCXX_ASSERTIONS for opting in. It's basically a library-only thing, much easier to implement than source fortification.
pornel 4y ago
I'm curious if this will get done and adopted. This is where "Modern C++ is a safe language" needs to walk the walk.
deleted 4y ago
[deleted]
ape4 4y ago
Could it detect the use of an iterator after a vector<> push_back()
lupbun 4y ago
This is great news. Such hardening can already be compiled in C code, and is even shipping in some projects, e.g. the Linux kernel has a CONFIG_UBSAN option which activates compiler sanitizers for things like array bounds checks that will panic on failure. Makes sense to extend similar behaviour to projects based on C++ codebases.

112 points by gok 4y ago

[dead]

I’m afraid this might be too little, too late. But regardless whether I’m right or wrong, we all should welcome a widely used language trying to become a little more safe.

yazzku 4y ago
On the other hand, it appears to have a great bang/buck. Link existing programs against the hardened libc++ and get those checks for "free" (other than the perf hit.)
- jeffbee 4y ago
  I'm not at all clear on how exactly that would work. The entire program would have to be rebuilt from scratch, would it not? Things like std::vector::operator[] are not runtime library calls, they are almost always inlined by the compiler. You could not simply take an existing program, replace its runtime library, and get bounds-checked vector accesses.
  - pjmlp 4y ago
    Depends how the linking took place, and optimization levels.
    - planede 4y ago
      vector is a template, at most vector<some_type>::operator[] will be a weak symbol, and I bet it's inlined from -O1 and above.
      - gpderetta 4y ago
        Even if not inlined at -O0, when instantiated for a custom some_type, the standard library .so won't contain any code for it, everything will be in the application binary.
        pjmlp 4y ago
        Depends on the implementation of helper functions and if external templates are being used.
        gpderetta 4y ago
        extern templates won't help for user defined types of course as the standard library couldn't possibly have instantiations for them :). Some non template helpers might be improved of course.
        pjmlp 4y ago
        They help for common types like std::string.
      - pjmlp 4y ago
        On Windows it can still live on a dynamic library. That has been a thing since the Windows 16 bit days, and other OSes with similar linking models like Symbian, OS/2 and AIX.
        jeffbee 4y ago
        It’s difficult to imagine which applications would find the performance of an outline vector access acceptable. Generally vector::operator[] is represented in built programs by a single x86 mov. A call would wreck the performance. In languages with bounds-checked vectors, they don’t do it with library calls.
        pjmlp 4y ago
  - gpderetta 4y ago
    Of course, it is a compiler feature, you will have to recompile and manually (or automatically) apply all suggested changes to the source code. If you just link against an hardened library, only the implementation of the (non inlined) library calls will be hardened, the user code itself won't see much benefits. Of course these days we have binary level optimizers, so who knows what it is possible in principle?
  - yazzku 4y ago
    Sorry, I used 'link' very liberally there, but you cleared it up. My point was that (from what I remember, it's been a few days) you'd mostly just need to recompile. As opposed to, you know, Rewrite It In Rust (TM).
green_on_black 4y ago
I agree. But the butthurt side of me wants to say: let those who wish for unsafety be unsafe.
- atoav 4y ago
  But programming is multiplication. A programmers work multiplies. If some person once writes code that ends up leaking users data, that data might like a thousand users data in a way that cannot be unleaked. That broken code then multiplies through time. You would not say: Let those [civil engineers] who like unsafe bridges build unsafe bridges Because the people using them might not be able to judge the unsafety of the system. Because the work of programmers multiply we have a huge responsibility and a huge chance to improve the world.
  - rapidlua 4y ago
    But c’mon there’s no such thing as safe brain surgery. Would you then market brain surgeries as unsafe?
    - rurban 4y ago
      Actually working on safe brain surgery robots. It's marketed as safe, and the code is checked to be hard-realtime and safe.
    - yakubin 4y ago
      Yes? I wouldn’t even think that’s in any way controversial.
    - samus 4y ago
      Bad comparison. Brain surgeries are a method of last resort. They are unsafe because there are significant knowledge gaps in our understanding of the brain that might take centuries more to fill. Every time a neurosurgeon cuts, they are potentially doing irreversible damage to an important part of a person's consciousness, abilities and memories. Edit: and there is no way to prevent that damage except by detailed planning and by stopping to cut when the patient (under local anestesia) ceases talking or performing their craft. Other damage can only be detected later though.
pjmlp 4y ago
Given that in many domains C++ will keep being the language to use, and most wannabe replacements depend on GCC and LLVM infrastructure (written in C++), any little bit of improved safety helps.
rychco 4y ago
Too late for what? C++ will continue to be used for decades.
- scrubs 4y ago
  If C++ could, and I think with some respect and acknowledgement to others as having gotten it better, adopt two improvements: * library/language side: some domain of bullet proof memory * compiler/language side: move build dependencies into the compiler or something that frontends the compiler ala GO/Rust with a flag to enforce it so it's optional. Here don't let perfect be the enemy of the good. There'd need be some new keywords or namespaces to make this work. That'd be superb I have all kinds of chances and opportunities to learn Rust but our eco system is endless C/C++. So I'd rather not deal with another complicated language. There's no near term or medium term timeline to move even 20% into Rust. I bail out to GO for I/O bound work.
  - maldev 4y ago
    There's multiple frameworks which give bullet proof memory usages and other fixes through annotations. The most common being https://learn.microsoft.com/en-us/cpp/code-quality/understanding-sal?view=msvc-170 https://learn.microsoft.com/en-us/cpp/code-quality/understan... . People just dont use them besides kernel devs, and even then they don't really use it. I believe the entire MS kernel is written using this.
    - pianoben 4y ago
      a) Not just the Windows kernel - Office uses SAL, too, in numerous places. Mostly in legacy places predating more modern C++. b) It's not true to say that kernel devs don't really use it. They use it a lot, though there are attempts at obsoleting some of it[0]. [0]: https://github.com/microsoft/wil https://github.com/microsoft/wil
      - maldev 4y ago
        Oh nice, I didn't know that about office, it makes sense though. I don't work for Microsoft and mainly work in the Antivirus space, so I only occasionally hear stories from friends about their internal stuff. When I said driver devs don't always use it, I mainly meant third party vendors writing kernel code. I know alot of the overseas places don't, and I've rarely had jobs which use it.

deleted 4y ago

[deleted]

yazzku 4y ago

saagarjha 4y ago

> For example, accessing a std::span or a std::vector outside of its bounds would abort the program, and so would accessing an empty std::optional. This can be done while staying Standards-conforming because undefined behavior implies that the library can do whatever it wants, which includes aborting. sickosyes.jpg Getting bad operations to abort execution (e.g. by throwing an exception) seems impossible to ship, not in the least due to certain, uh, entities really being against it. But abusing UB to allow people to turn on checks is excellent. This is why we should be very careful about what we choose to standardize-a common request is making signed overflow wrap instead of being UB, which prevents its diagnosis in this way.

olliej 4y ago
Making out of bounds access on explicitly bound types is an easy memory safety win. The fact the UB is still being used as an excuse to clearly undermine reasonably expected behaviour remains a BS trope, and WG21's continued acceptance of new features and specifications that say perfectly specifiable behaviour is UB instead of unspecified is a continuing source of avoidable security bugs. It is inexcusable for new features to be added to C++ that have UB in any case other than the undefinable (invalid memory access, etc). Removing the copious unnecessary UB from the existing specs should be a higher priority that many of the new features being proposed.
- saagarjha 4y ago
  Much that is UB deserves to be defined. And some that is defined deserves to be UB. Can you make it illegal? Then do not be too eager to deal out behaviors in standardization. I don't actually oppose, in the long run, slowly picking some behaviors and changing them to be standardized. But I do vehemently oppose many of the suggestions that are brought up. The kinds of things that should be moved out of being undefined are things like "you named your function isfoo" and not "you passed NULL to memset".
  - olliej 4y ago
    > And some that is defined deserves to be UB. Can you provide an example of currently specified behaviour that should be UB? To be very clear I am using specification language here. "Unspecified behaviour" is very much not "undefined behaviour".

rapidlua 4y ago

This is unfortunately ABI-altering, therefore it won’t be possible to selectively enable for projects that want it. It’s a distribution that has either to bite the bullet and say “we are going to pessimize all C++ software”, or err on the safe side and leave it disabled. The warning about pointer arithmetic is most likely suppressable per compilation unit (for things like tagged pointers).

olliej 4y ago
What is the ABI altering concern? Switching from pointer+length->span? Because yeah that is alas a source of suffering. > “we are going to pessimize all C++ software” Could you clarify?
- rapidlua 4y ago
  As of today, vector’s iterator is just a pointer. You need more than just a pointer to detect out of bounds accesses. It makes old binaries incompatible with new binaries. It also means that you can’t mix and match, it must be either a or b. Hence you can’t make the change incrementally, which is a big risk. Concerning Linux distributions, I seriously doubt if they have resources to assess the performance impact across the wide range of software. Concerning performance impact, we end up with more instructions and more memory accesses and increased register pressure. This has a chance to make things slower.
  - pabs3 4y ago
    Perhaps you could keep ABI compatible using with versioned symbols?
    - gpderetta 4y ago
      If the functions are separate overloads there is no need of versioning in the first place. On the other hand if you change the layout of structures (like it would be the case for bound checked pointers), it is much much harder. GCC has ABI tags, but they only help a bit. In any case nobody is going to bound check iterators by default, these already exists when using whatever debug standard library your compiler provides and they are orders of magnitude slower than unchecked iterators.

gpderetta 4y ago

If I'm reading this correctly, this has two components: - a bound checked libc++, which is not particularly exciting, libstdc++ has had _GLIBCXX_ASSERTIONS for a while for example. In fact it seems that libc++ has had it as well. - clang warnings to flag potentially unsafe code plus fix-it suggestions to convert it to safer code using the bound checked libc++. The later is quite interesting, especially the fix-it might make it easier to incrementally "harden" a large c++ code base.

rwmj 4y ago
The first one also sounds a bit like -D_FORTIFY_SOURCE=2 in glibc which fortifies memcpy etc. Many Linux distros have had this enabled by default for years. (https://stackoverflow.com/a/16604146 https://stackoverflow.com/a/16604146) Agree the second one both sounds interesting and also quite disruptive to existing code.
- fweimer 4y ago
  _GLIBCXX_ASSERTIONS is slightly different because it uses the bounds information that is already available. The historical expectation is that operator[] doesn't perform bounds checking by default, you need to use the at(size_type) members for that. Hence _GLIBCXX_ASSERTIONS for opting in. It's basically a library-only thing, much easier to implement than source fortification.

pornel 4y ago

I'm curious if this will get done and adopted. This is where "Modern C++ is a safe language" needs to walk the walk.

deleted 4y ago

[deleted]

ape4 4y ago

Could it detect the use of an iterator after a vector<> push_back()

lupbun 4y ago

This is great news. Such hardening can already be compiled in C code, and is even shipping in some projects, e.g. the Linux kernel has a CONFIG_UBSAN option which activates compiler sanitizers for things like array bounds checks that will panic on failure. Makes sense to extend similar behaviour to projects based on C++ codebases.