7 ms·

C++ Software Security Sins: Basic Issues

57 points by 0xedb 5y ago

zabzonk 5y ago
No even vaguely competent C++ programmer uses arrays of char such as char a[26] - this is completely bogus.
- spacechild1 5y ago
  It is clearly meant as a bad example, followed by suggestions on how to fix it. Replace “26“ by some #define or const variable and then tell me again you haven't seen code like this. Not to mention that this article is clearly aimed at beginners.
- pjmlp 5y ago
  You would be surprised how the corporate world delivers C++ code, specially those projects with offshoring.
- hknapp 5y ago
  what do you use? std::array? alloca? std::string? heap allocation? a lot of std library functions need char *
  - bierjunge 5y ago
    std::array<char, 26> foo{}; foo.data(); // <- returns a pointer to foo's data inheriting it's type. so we get *char here std::string would work in a similar way, except it would be `const`
- tialaramex 5y ago
  But why though? I mean, if I would like exactly 26 chars, this seems a nice way to get that. Even understanding that in C++ "char" is just a weirdly archaic way to say "byte" I might actually want that. Obviously it's bogus for the language to let you say you want an array of 26 chars and then treat that as a reasonable place to write an unknown number of characters, but that's a language fault.
  - duped 5y ago
    I'm guessing the reason is because `std::array<char, 26>` gives you everything you'd want from `char[26]` but with more bells and whistles.
    - tialaramex 5y ago
      Sure, but that's a bug right? The language isn't able to give you those bells and whistles for its actual array type, so as a work around here's something else instead. Naming it "array" won't fool anybody when your language does have arrays already. I mean, given what is intended in the examples is a string, obviously you should use an actual string. But if you want an array, you should be able to use an array for that, and it's silly that instead you're asked to use a substitute.
      - duped 5y ago
        This may be splitting hairs, but `std::array` is how the language provides bells and whistles (via the STL). You can see here (1) it's a transparent wrapper around a raw array. (1) https://github.com/microsoft/STL/blob/62137922ab168f8e23ec1a95c946821e24bde230/stl/inc/array#L597 https://github.com/microsoft/STL/blob/62137922ab168f8e23ec1a...
        tialaramex 5y ago
        On the one hand, thanks for the terrifying code. An earlier thread on HN left me wading through Rust internals which are very ugly, but this is reassuringly worse. But on the other hand, I already am splitting hairs. C++ comes with actual arrays, which lack bells and whistles, adding the bells and whistles to a class named "array" in the STL instead does not add them to actual arrays. If it did that would be a fine fix, but it doesn't. Rust users who've told the compiler that their function wants, for example, an array of 26 chars can cheerfully assume in the function that the array has 26 chars in it. The compiler won't let anybody call that function with anything else so it'll be fine. It can make sense to do that, although perhaps not with exactly 26 chars. C++ users can write a pretty similar looking function signature, including that size constraint - but, a C++ compiler just ignores the integer 26 and silently gives them a pointer to what may or may not be some number of chars, even though that's clearly not what the author intended. They need to write something quite different if they want the compiler to look after this constraint, and if they knew to do that they wouldn't be in this mess in the first place.
- tobiasSoftware 5y ago
  I've seen that in professional decently built code. In the code that I am thinking of though, instead of 26, it is MAX_PATH, which is 260 and is used to specify the maximum length of a filepath. It is used to interface with specific Microsoft functions that handle file paths by taking in an array of chars and immediately after is converted to a std::string. Sure, it could be dynamically allocated, but if it gets a filepath longer than the maximum length of a filepath, then it is already dealing with some sort of undefined behavior.
hedora 5y ago
Various smart pointer types solve the uninitialized pointer problem. You can even have variants that are guaranteed to be non-null (unlike, say, java, where null references can escape during initialization, especially in multithreaded code)
MauranKilom 5y ago
This blog post feels weirdly superficial (haven't looked at the video/presentation). > Array new and delete > When you write new in your applications, you are creating unmanaged objects, and you are then required to call delete later on if you don’t want to risk leaks. So don’t use new and delete at all, as this is considered a C++ bad practice. Better yet, working in modern C++ allows you to use smart pointers and Standard library container classes that make it easier to match every new with exactly one delete. The text is reasonable, but why is the heading "array new and delete"? There is a difference between new/delete and new[]/delete[], and calling delete[] on something created by new or vice versa is indeed going to cause problems. But that problem (and hence the section title) is more or less orthogonal to what the section body talks about. Edit: The corresponding slide in the presentation does talk specifically about this kind of mismatch. It also comes to the same conclusion as the text ("just use the STL"). The connection between title and body was apparently lost in translation. Maybe as more general feedback, I'm getting a strange vibe from this blog. Lots of "top 5 <C++ thing>" posts, links to Patreon-gated articles, ads for blog-owner-written books... Which would all be fine, but that plus an inconsistent summary of someone else's talk raises a lot of warning signs for me.
jahnu 5y ago
For any sufficiently mature programming language there exists older patterns that should be avoided but remain possible.
- tialaramex 5y ago
  As I understand it this is one purpose of Rust's "Editions" behaviour. They can deprecate something, suggesting what you ought to do instead, and then in a subsequent edition they can actually outlaw it. Your old code still compiles, since that was for the earlier edition, and editions compose in both directions perfectly well so that the ecosystem isn't fragmented, but new projects will be written for a newer edition where the deprecated code now does not compile, and so the deprecated practice actually dies out in the minds of programmers, rather than limping on forever because of doubts about backward compatibility. Rust is a young language, perhaps in twenty years we'll know that editions weren't powerful enough, but it seems like a good start. This year will see a new edition (Rust 2021) with a handful of such changes, such as requiring you always write 1..=9 not the historical alternative 1...9 when you mean that the range should be exactly 1,2,3,4,5,6,7,8,9.
malaya_zemlya 5y ago
weirdly, this article advises to use unsigneds for array offsets, which is a fine way to overwrite the heap - when you accidentally decrement an array index past 0.

57 points by 0xedb 5y ago

zabzonk 5y ago

No even vaguely competent C++ programmer uses arrays of char such as char a[26] - this is completely bogus.

spacechild1 5y ago
It is clearly meant as a bad example, followed by suggestions on how to fix it. Replace “26“ by some #define or const variable and then tell me again you haven't seen code like this. Not to mention that this article is clearly aimed at beginners.
pjmlp 5y ago
You would be surprised how the corporate world delivers C++ code, specially those projects with offshoring.
hknapp 5y ago
what do you use? std::array? alloca? std::string? heap allocation? a lot of std library functions need char *
- bierjunge 5y ago
  std::array<char, 26> foo{}; foo.data(); // <- returns a pointer to foo's data inheriting it's type. so we get *char here std::string would work in a similar way, except it would be `const`
tialaramex 5y ago
But why though? I mean, if I would like exactly 26 chars, this seems a nice way to get that. Even understanding that in C++ "char" is just a weirdly archaic way to say "byte" I might actually want that. Obviously it's bogus for the language to let you say you want an array of 26 chars and then treat that as a reasonable place to write an unknown number of characters, but that's a language fault.
- duped 5y ago
  I'm guessing the reason is because `std::array<char, 26>` gives you everything you'd want from `char[26]` but with more bells and whistles.
  - tialaramex 5y ago
    Sure, but that's a bug right? The language isn't able to give you those bells and whistles for its actual array type, so as a work around here's something else instead. Naming it "array" won't fool anybody when your language does have arrays already. I mean, given what is intended in the examples is a string, obviously you should use an actual string. But if you want an array, you should be able to use an array for that, and it's silly that instead you're asked to use a substitute.
    - duped 5y ago
      This may be splitting hairs, but `std::array` is how the language provides bells and whistles (via the STL). You can see here (1) it's a transparent wrapper around a raw array. (1) https://github.com/microsoft/STL/blob/62137922ab168f8e23ec1a95c946821e24bde230/stl/inc/array#L597 https://github.com/microsoft/STL/blob/62137922ab168f8e23ec1a...
      - tialaramex 5y ago
        On the one hand, thanks for the terrifying code. An earlier thread on HN left me wading through Rust internals which are very ugly, but this is reassuringly worse. But on the other hand, I already am splitting hairs. C++ comes with actual arrays, which lack bells and whistles, adding the bells and whistles to a class named "array" in the STL instead does not add them to actual arrays. If it did that would be a fine fix, but it doesn't. Rust users who've told the compiler that their function wants, for example, an array of 26 chars can cheerfully assume in the function that the array has 26 chars in it. The compiler won't let anybody call that function with anything else so it'll be fine. It can make sense to do that, although perhaps not with exactly 26 chars. C++ users can write a pretty similar looking function signature, including that size constraint - but, a C++ compiler just ignores the integer 26 and silently gives them a pointer to what may or may not be some number of chars, even though that's clearly not what the author intended. They need to write something quite different if they want the compiler to look after this constraint, and if they knew to do that they wouldn't be in this mess in the first place.
tobiasSoftware 5y ago
I've seen that in professional decently built code. In the code that I am thinking of though, instead of 26, it is MAX_PATH, which is 260 and is used to specify the maximum length of a filepath. It is used to interface with specific Microsoft functions that handle file paths by taking in an array of chars and immediately after is converted to a std::string. Sure, it could be dynamically allocated, but if it gets a filepath longer than the maximum length of a filepath, then it is already dealing with some sort of undefined behavior.

hedora 5y ago

Various smart pointer types solve the uninitialized pointer problem. You can even have variants that are guaranteed to be non-null (unlike, say, java, where null references can escape during initialization, especially in multithreaded code)

MauranKilom 5y ago

This blog post feels weirdly superficial (haven't looked at the video/presentation). > Array new and delete > When you write new in your applications, you are creating unmanaged objects, and you are then required to call delete later on if you don’t want to risk leaks. So don’t use new and delete at all, as this is considered a C++ bad practice. Better yet, working in modern C++ allows you to use smart pointers and Standard library container classes that make it easier to match every new with exactly one delete. The text is reasonable, but why is the heading "array new and delete"? There is a difference between new/delete and new[]/delete[], and calling delete[] on something created by new or vice versa is indeed going to cause problems. But that problem (and hence the section title) is more or less orthogonal to what the section body talks about. Edit: The corresponding slide in the presentation does talk specifically about this kind of mismatch. It also comes to the same conclusion as the text ("just use the STL"). The connection between title and body was apparently lost in translation. Maybe as more general feedback, I'm getting a strange vibe from this blog. Lots of "top 5 <C++ thing>" posts, links to Patreon-gated articles, ads for blog-owner-written books... Which would all be fine, but that plus an inconsistent summary of someone else's talk raises a lot of warning signs for me.

jahnu 5y ago

For any sufficiently mature programming language there exists older patterns that should be avoided but remain possible.

tialaramex 5y ago
As I understand it this is one purpose of Rust's "Editions" behaviour. They can deprecate something, suggesting what you ought to do instead, and then in a subsequent edition they can actually outlaw it. Your old code still compiles, since that was for the earlier edition, and editions compose in both directions perfectly well so that the ecosystem isn't fragmented, but new projects will be written for a newer edition where the deprecated code now does not compile, and so the deprecated practice actually dies out in the minds of programmers, rather than limping on forever because of doubts about backward compatibility. Rust is a young language, perhaps in twenty years we'll know that editions weren't powerful enough, but it seems like a good start. This year will see a new edition (Rust 2021) with a handful of such changes, such as requiring you always write 1..=9 not the historical alternative 1...9 when you mean that the range should be exactly 1,2,3,4,5,6,7,8,9.

malaya_zemlya 5y ago

weirdly, this article advises to use unsigneds for array offsets, which is a fine way to overwrite the heap - when you accidentally decrement an array index past 0.