7 ms·
By my count 12 out of the 14 vulnerabilities in this advisory are due to manual memory management. And past ones were similar. This is security critical code,
by BoppreH 12y ago
By my count 12 out of the 14 vulnerabilities in this advisory are due to manual memory management. And past ones were similar.
This is security critical code, being developed and polished for more than a decade, and the only thing protecting the privacy of millions (billions?) of people. If any public project should have gotten this right, it should be this. Yet it can't.
Is this time to admit we simply can't do manual memory management at this level and move to something else? I know the performance boost is tempting, but I think we can't afford the consequences.
- dtech 12y agoIt's mainly due to OpenSSL doing exceptionally poor manual memory management. Lot's of the LibreSSL changes have involved removing that.
- BoppreH 12y agoFlash Player, Adobe Reader, JVM, etc etc etc. I don't think it's a localized problem. And we only have so many people with the skills of the LibreSSL team to go around.
- dtech 12y agoSo you're suggesting that the solution to manual memory managment bugs in the JVM is to use a memory managed language? The memory management will have to be done manually at some point.
- rurban 12y agoOpenSSL has not even a testsuite or build tool helpers to help finding those simple bugs, usually found with normal build and test methods, such a valgrind or AddressSanitizer. Insanity prevails.
- stephen_g 12y agoPart of the problem was that they were (are?) using their own memory allocator, which was not only had been the source of many security vulnerabilities, but also broke Valgrind! That was one of the first things the LibreSSL team ripped out.
- tomjen3 12y agoOpenSSL is just bad code. You can keep a reference around to memory in every garbage collected code. What OpenSSL needs to do is to throw away all the extra code that makes it run on a billion obsolete platforms, remove support for things that aren't needed (renegociating encryption algorithms? Really if a client supported it before it can support them now), wrap the internal parts of the library and port it to C++ so that they can use smart pointers, then ban malloc. Manual memory management isn't dangerous, malloc and low level primitives are (e.g you should never manually append strings, other than in your string library, which should automatically resize the strings as needed).
- Someone1234 12y ago> Manual memory management isn't dangerous, malloc and low level primitives are That is somewhat self-contradictory. Those "low level primitives" are manual memory management, so even by your own admission it is dangerous. Even Wikipedia calls smart pointers "automatic memory management."
- AnthonyMouse 12y agoThere is a distinction to be drawn between a) malloc/free, b) RAII as with Rust or modern C++ and c) garbage collection. We can have a debate about whether RAII is "manual memory management" or not, but it's clearly safer than malloc/free without the inefficiency of garbage collection.
- bandrami 12y ago>> What OpenSSL needs to do is ... port it to C++ Awful idea. The absolute last thing we want is for memory management to be out of OpenSSL's explicit control (then we would have two audit trails to worry about). Automatic memory management is manual memory management you've outsourced to somebody else. (By this same argument, you also shouldn't turn on compiler optimizations when you build openssl.)
- sporkenfang 12y agoIt's not the language, it's the programmers (and the lack of proper/regular code review).
- ctz 12y agoYou'll also notice that exactly none of the problems fixed in this advisory have any tests, either to check for regressions or for similar problems elsewhere. This is just negligent.
- sporkenfang 12y agoI would agree. (I already do, but) I wish more people who understood the importance of testing what you build would contribute to projects like this!
- amirmc 12y ago> "Is this time to admit we simply can't do manual memory management at this level and move to something else? I know the performance boost is tempting, but I think we can't afford the consequences." People have built a clean-slate TLS stack in OCaml and you can read about it on the MirageOS site [1]. There's also a Bitcoin Piñata built using this stack [2]. I'm told the performance is actually pretty good (though there's always room for improvement). [1] http://openmirage.org/blog/introducing-ocaml-tls http://openmirage.org/blog/introducing-ocaml-tls [2] http://amirchaudhry.com/bitcoin-pinata/ http://amirchaudhry.com/bitcoin-pinata/ and https://news.ycombinator.com/item?id=9027743 https://news.ycombinator.com/item?id=9027743
- CCs 12y agoGC is not an answer to everything; sometimes works, sometimes you need to shortcut it. Many people work on manual memory management for a reason http://event.scaladays.org/scaladays-sanfran-2015#!#schedulePopupExtras-6560 http://event.scaladays.org/scaladays-sanfran-2015#!#schedule... Security is a practice, not a programming language. Anthem uses C# and they are hacked almost every year. https://www.anthemfacts.com/ https://www.anthemfacts.com/
- ploxiln 12y agoIt's not been "polished" for more than a decade, it's been "patched" (for both bugs and features). Perhaps even "trashed". OpenBSD uses a whole lot of C and doesn't tend to have these problems... I'm not at all sympathetic to calls to ban some programming tool because it's "too hard". Just stop trying to squash harsh criticism as "toxic culture", this is how you imbue in people the importance of being careful with important code. Otherwise, even if they acknowledge that they should be careful, they'll write sloppy code, and it might even become popular, and you might have to support it...
- userbinator 12y agoI'd say the problem is in the documentation (or lack thereof.) I've had to use OpenSSL before and the question of "should this thing be freed after I use it?" was very common and only resolved fully by inspecting the source carefully. The heavy use of macros adds to the frustration; often, 3 or more layers of them hide the actual code that gets executed, making it hard to tell where things go. On the other hand, I've also worked on some codebases that were very well-documented with respect to this: every function that either returned a pointer or accepted pointers made it clear if/when they should be freed (and if so, with what function.) One example of that looks like: void /* OWN */ *some_func(void *a, void /* OWN */ *b, void */* OWN */*c); And the "/* OWN */" are intended to be read like "const" and "static", so this says the function returns a pointer to an object whose ownership is transferred to the caller, its first parameter does not change owners, the second one does, and the third is a pointer to an array of pointers whose ownership is passed to the function. The accompanying comment then says what function should be used to free the returned pointer, and what function will be used (by other code) to eventually free b and c.
- maemre 12y agoI know that proposing Rust in this case almost became a cliché but Rust's borrowing mechanism introduces such a semantic and ensures correctness in compile time. Moreover, with RAII it can also zero the memory after object is out of scope.
- _RPM 12y agothat's why we use LibreSSL
- pcunite 12y agoOpenSSL is how not to do an important project. It is not indicative of C, opensource, underfunding, or any of the other common reasons normally given. It is the result of too many cooks in the kitchen. Some of who in fact are not cooks.
- FrankBooth 12y agoI find it more apropos to suggest that they're all cooks, but meth cooks.
- MichaelGG 12y agoBut many of these null pointer dereferences would _also_ be a DoS in a memory-safe, exception-free language like Rust. If you think x isn't null then try to access x.y... something's gonna blow up. If it was in Rust and someone used an unwrap() but there was no value, panic would DoS the system in much the same way, right? (Same for Java, or C#, or what have you, though each connection would probably be in a try/catch so no real harm done, most likely.)
- masklinn 12y ago> If it was in Rust and someone used an unwrap() but there was no value, panic would DoS the system in much the same way, right? A big difference is that the developer would have made the explicit choice at that point to dereference a pointer know to possibly be null. There's no implicit null (and having to wonder "by hand" for each pointer whether or not it could be null, with the risk that changes in any previous operation make make non-nullable pointers nullable without warning). > Same for Java, or C#, or what have you Java and C# also use implicit pervasive nullability, and at least deref'ing a null pointer there is a defined behavior.