Comment by q3k
5 years ago
As much as I like to bash security critical code written in memory-unsafe languages, I don't think that this is the crux of the problem here.
To me it's that this extremely trivial bug (the heap overflow, let's ignore the rest for now) passed through code review, security review, security audits, fuzzing... Or that Apple didn't have these in place at all. Not sure which option is worse.
We have 30 years of experience showing that ordinary heap overflows are not in fact easy to spot in code review, security review, security audits, and fuzzing. Each of those modalities eliminates a slice of the problem, and some of them --- manual review modalities --- will remove different slices every time they're applied; different test team, different bugs.
To me, this strongly suggests that the problem is in fact memory-unsafe languages, and not general engineering practices.
Apple, by the way, has all the things you're talking about in place, and in spades.
I agree that the problem is memory-unsafe languages.
You can improve the tools, or you can improve the human, and nobody has managed to improve the human despite decades of trying.
OTOH, we don't really have evidence to show that memory safety is effective in kernels/drivers because no memory safe language has ever been deployed at scale for that purpose.
The way I look at it is that relying exclusively on manual review is at best the same as relying on both manual review and a memory safe language.
In practice, the best case and average case rarely line up.
You don't have to manually review for classes of vulnerability that your programming environment forecloses on.
1 reply →
> the problem is in fact memory-unsafe languages, and not general engineering practices.
Languages don't introduce bugs by themselves. Engineers produced those bugs.
I always thought that bugs are the programmers' fault, and not to blame the language. It's like blaming the English language because it allows you to misuse it and manufacture very offensive racial slurs, or to be rude and cruel, and thus we should replace it with another language that doesn't allow to exploit these weaknesses. We won't be able to express ourselves with beautifully (low-level) crafted poems anymore, but that's the price to pay.
There are inherent features of human languages that force you into weird issues. English for example has gender pronouns and that's why you see in profiles how you should approach someone. It's not like they want to add it, it's that they have to if it bothers them when people misuse them.
In Hungarian we don't have this problem at all, there's no concept of gender specific pronouns.
Such bugs are extremely difficult to prevent at scale. Even the most talented engineers make such mistakes and programming quality varies significantly even within top engineering teams which are usually comprised of people with different skill sets (+ junior engineers that need training).
Safe languages are the only way forward to drastically reduce the problem. It can’t be guaranteed to be eliminated 100% obviously because there are still escape hatches available, but it will be significantly improved because you can limit the surface area where such bugs can live.
Not sure if this is common for everyone but I find whenever I get assigned for a review for a monster change, I spend over an hour just working out what the change does and if it seems like it will work. There is no way I could spot tiny potential security exploits in 3000 lines of changed code.
Sending 3,000+ LoC code reviews is not considered good engineering practice. In general it's important to keep code reviews small for the reasons you describe. If it's impossible to make incremental changes in smaller units, that's a sign of an unhealthy code base.
Although it is certainly challenging, you shouldn't spend only an hour reviewing a 3,000 LoC review, unless most of it is trivial refactoring, especially if that code is security-critical and handles untrusted input. That's only 1.2s per line of code. No chance you have good quality control with that amount of attention.
If it's taken someone a whole fortnight to write it, you should expect to spend at least half a day reviewing it, IMO.
Does any software producer do fuzzing on their own product? I have never heard of this being done by software developers. Usually it's done by exploit developers. Of course there are static analysis tools that should uncover a problem like this, and I know that high-reliability embedded software developers use them, but I don't know if the likes of Apple does.