8 ms·
I looked at the offending function, and it would be trivial to rewrite it in C++ in an easy to understand and safer way. It probably wouldn't be as safe as Rust
by blub 11y ago
I looked at the offending function, and it would be trivial to rewrite it in C++ in an easy to understand and safer way. It probably wouldn't be as safe as Rust, but still a lot better.
A lot of Linux C utilities would benefit from such a treatment.
- joushou 11y agoA rewrite in C++ would not make it any safer. Rewriting it to make it easier to understand could make it safer, but you do not need a different language for that.
- blub 11y agoYes it would, because this is basically string processing where C is the worst possible language one could use. I've detailed in another comment how std::string could be used.
- joushou 11y agoYou're talking about naked char[]. You can just as easily make a struct{ int len; char* str; } in C, and combined with the "n" variants of string operations, would work just fine with common tools. Again, C++ does not make anything safer, and its types can easily be replicated in any other language.
- kccqzy 11y agoI believe this is called pascal style strings and GCC has builtin support for it.
- pjmlp 11y agoBeing possible, doesn't mean the majority of C developers make use of it. The "n" variants are a joke in terms of security, even the C99 annex, that was demoted to optional in C11. I call them a joke, because tracking the pointer and length separately is hardly an improvement in terms of security. The only improvement that the "n" variants added is that the null character is always added to the end, instead of how strncpy does it, by only adding the character if there is enough space.
- NoGravitas 11y agoYou could stay in C and use glib strings.
- claudius 11y agoIf you can change the name_path struct to a standard STL data structure (std::vector<std::string> might work just fine), you can easily rewrite the function such that all memory allocation and all exploitable memory access is delegated to STL containers. If your STL std::string has a buffer overflow issue, you’re of course still in trouble, but the same is true if there are bugs in the STL/implementation of Go/Rust/Python/Java/Haskell… Something like: std::string path_name(std::vector<std::string> const& dirs, std::string const& name) { std::string p; for(auto const& dir : dirs) { p += (dirs + "/"); } p += name; return p; } should work. If you feel like it, you can also add something like: std::size_t len = name.size(); for(auto const& dir : dirs) { len += dir.size(); } p.reserve(len); Of course, len may overflow, but even if it does, all the harm that causes is that the string will have to reallocate memory during growth until running in a segfault when further memory allocation fails.
- smitherfield 11y agoI think you have a typo in your first code block. p += (dir + "/");
- claudius 11y agoOh, yes, sorry and now it’s too late to fix (both that and the horrible formatting). Though I suppose it would have been a compile-time error, so at least it shouldn’t be exploitable :')
- smitherfield 11y agoNo worries! :)
- joushou 11y agoRight, and you can do the same with C, using libraries providing this functionality wrapped as well. There's plenty of std::string and std::vector like containers, which handle the magic for you. But even then, you can work with struct {len int; void* data;} to get your vector, and replace void* with char* to get a string. A simple vector and simple string is in no way difficult to implement, and many implementations exist. I'm just trying to point out that the convenience of some C++ standard library features is not isolated to C++, and C++ is not a "memory safe" language by meaning of the word.
- lmm 11y agoIf you're going to rewrite in any case surely it's worth going to Rust (or Ada, or another language that simply doesn't have the myriad unsafe possibilities that C++ does).
- blub 11y agoThere are many arguments to be made in favor of C++, such as talent availability, experience from other porting projects, the possibility of doing incremental porting, etc. My impression is that some projects are already experimenting with or using C++ in their C code bases, so C to C++ is quite likely.
- lmm 11y agoMy impression is that projects that mix C and C++ tend to use the C-like subset of C++ (with classes), so they wouldn't gain any safety advantage from moving to C++. Many other languages support C linkage in a way that's comparable to C++.
- blub 11y agoUsing std::vector, std::string and smart pointers would already bring large benfits, so even a C-like subset could work fine. The point is that code like in that function is a nightmare to write correctly and test in plain C.
- pjmlp 11y agoI agree with you, the only problem being that even in 2016 there are shops that prohibit the usage of STL. The real way out is to eventually change to a language where safety is opt-out and not opt-in, like in C++.
- w8rbt 11y agoC++ is far safer than C precisely for the reasons listed. The standard is safe by default (it's not opt-in). If you want or need backward compatibility with C, then you can use the more error prone C constructs for that. Otherwise, pure C++ code is safe.
- userbinator 11y agoRewriting in any language won't help with this one unless the language has builtin integer overflow checking. The solution here is actually quite trivial: just restrict filenames to 255 bytes and nesting to 255 levels, which limits paths to 64KB at most. Anyone trying to use git repositories exceeding either of those limits should be considered insane.
- trywerwerwer 11y ago> just restrict filenames to 255 bytes and nesting to 255 levels, which limits paths to 64KB at most. Anyone trying to use git repositories exceeding either of those limits should be considered insane. Funny, because I've actually been locked up for being insane five times. Speaking from experience, they'd only consider me insane if I said something like 255 was an important number because there are two sides to every problem and five fingers on each hand, and the path, which two feet take, is six one way / half a dozen the other, to the four corners of the earth, which is the natural limit. So... you don't really know what insane is. Jus' sayin'.
- chris_wot 11y agoAs someone who has also spent time in a mental institution, I just want to say that I really think you need to be less sensitive on this one. That's just an expression and not in any way aimed at people who have mental illness.
- blub 11y agoThe trick is to not do any index calculation. I think that using std::string would prevent the bug, because it would throw length_error on append. string::resize can be used to avoid excessive allocations. Sanity checks would provide extra safety, but the code should fail cleanly even without them.
- steveklabnik 11y agoSince we're talking about git, here's an interesting PR: https://github.com/libgit2/libgit2/pull/3540 https://github.com/libgit2/libgit2/pull/3540 (EDIT: Some stuff has changed since February, so some of these claims are now out of date. But not all of them, the overall situation is still similar.)
- kaffeinecoma 11y agoRe-writing Git in C++? Can't tell if you're trolling or not. http://harmful.cat-v.org/software/c++/linus http://harmful.cat-v.org/software/c++/linus
- pjmlp 11y agoYet given the actual state of Gtk development, even Linus is using C++ nowadays via Qt. https://github.com/torvalds/subsurface https://github.com/torvalds/subsurface
- geofft 11y agoArgument 1 is nontechnical and by someone who does not have a lot of experience writing userspace software. Argument 2 is valid, but only applies to kernels, and also only applies to 1992 (it works a lot better now).
- kaffeinecoma 11y agoMy comment wasn't meant to knock C++; it was to point out that Linus' opposition to the language is well-documented, and an official git-rewrite in C++ is very unlikely.
- geofft 11y agoLinus is almost entirely uninvolved with git these days, though. `git log --author=Torvalds` finds no commits in 2015 or so far this year, three patches in 2014, none in 2013, five in 2012, four in 2011... if the git core team felt using C++ was a possible course, Linus would not be involved in the decision. (That's probably why I misunderstoood your statement an argument against C++ on its own merits.)
- EGreg 11y agoLinus would chew you out for saying that son. http://harmful.cat-v.org/software/c++/linus http://harmful.cat-v.org/software/c++/linus