Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

'Heartbleed' contributor denies he inserted it deliberately

smh.com.au

Post reply on HN
Print view View original

61–70 of 86 posts

Re: 'Heartbleed' contributor denies he inserted it deliberately

#61

Post by nuncanada »

orblivion wrote: post #59
notacoward wrote: post #29

Maybe the silver lining here is that it puts the final nail in the coffin for "many eyes make all bugs shallow" - which was always total BS from the day it was uttered. There's so much code out there, much of it highly specialized and even project-specific, that there are very few eyes looking at any particular piece of code, and not all eyes are connected to the greatest of brains. Most static code analyzers could h…

> Most static code analyzers could have caught this particular bug Does that mean nobody out there in the crypto world ran a static analyzer on OpenSSL in the last 2 years?

OpenSSL's code uses particularly bad MACRO's ifdefs that defeat static analyzer's capabilities to analyze the code...

Re: 'Heartbleed' contributor denies he inserted it deliberately

#62

Post by mcherm »

nikcub wrote: post #41
krstck wrote: post #13

It we start naming and shaming the coder for each flaw instead of working on fixing the process that allowed it to sneak through, we'll see a chilling effect on open source software. There's a reason we have tests and code reviews and security audits...

I just happen to be speaking to the very reporter who wrote this article prior to him publishing this report. I told him that in 'our world' we don't really emphasize or make much of who introduced the bug, since it is understood that writing secure code is hard. But what required the denial from the developer (and I feel horrible for him) is that this bug gained so much mainstream attention and speculation. It was o…

> We all know that he didn't do it intentionally

Actually, I hadn't known that. But this article gave me strong reasons to believe that it was, indeed, accidental. I certainly recognize that this is the sort of mistake that is extremely easy to make.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#63

Post by notacoward »

nikcub wrote: post #33
notacoward wrote: post #29

Maybe the silver lining here is that it puts the final nail in the coffin for "many eyes make all bugs shallow" - which was always total BS from the day it was uttered. There's so much code out there, much of it highly specialized and even project-specific, that there are very few eyes looking at any particular piece of code, and not all eyes are connected to the greatest of brains. Most static code analyzers could h…

> Maybe the silver lining here is that it puts the final nail in the coffin for "many eyes make all bugs shallow" The OpenSSL project doesn't seem to have a 'review' step[0] in code commits, unlike FreeBSD, OpenBSD and Chromium (i'm just picking projects i'm familiar with, no bias). So it might not have been 'many eyes', but the eyes of just one. edit: [0] a proper review step, what the openssl project does is have f…

The "many eyes" canard isn't just about formal reviews, though. It's also about the idea that when something's open source the reviews aren't limited to maintainers or close colleagues who would be involved regardless of open vs. proprietary. What actually seems to happen in the great majority of cases is that the circle of people who actually understand any particular commit is only very slightly greater than it would be otherwise. The bugs are only a very tiny bit shallower, if at all. The fact that it was just one reviewer in this case, despite the code being widely available, only underscores that point.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#64

Post by phazmatis »

danielweber wrote: post #50
talmand wrote: post #24

Earlier quoted context omitted.

Unfortunately the tech industry seems to be in mob mode recently so I would think the chilling effect is already well underway. The fact that it has been suggested this was done intentionally before they know the whole story suggests this to me. Never mind the fact that the code was apparently reviewed, so I guess the reviewer would have been in on it as well. There are always people who prefer to play the blame game…

Part of me wants to see the team give the industry a big middle finger and quit the project, and now companies relying on it have to start spending money to keep it up instead of expecting it to be provided for free. If something is important to you, you should spend more resources than zero on it.

That would be great. Maybe we could get some TDD people in there. Some competent c engineers, rather than code cowboys.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#65

Post by phazmatis »

ajanuary wrote: post #56
fournm wrote: post #45

Earlier quoted context omitted.

I'm not familiar with Coverity, but Sonar (for the Java world) is very similar in concept and will fail to detect things if there are enough issues before it in a file... Which I would HOPE is not the case here.

From my experience with static analysis tools it's very easy to write perfectly valid code that the tool doesn't like, so it wouldn't surprise me too much. Of course, the argument can often be made that if it isn't clear enough for the tool to find, it's not clear enough for a person to understand quickly.

So you refactor until the noise goes away. Why throw out the signal with the noise? Modern software engineering theory 101.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#66

Post by senorprogrammer »

kmkemp wrote: post #20
Karunamon wrote: post #11

Earlier quoted context omitted.

The armchair crypto brigade aren't helping much, either, by passing around these completely unfounded accusations of malfeasance. I'm all for being aware of the possibility that shenanigans are involved, but until proof comes out, it is nothing but a possibility, and a remote one at that.

"Never attribute to malice that which is adequately explained by stupidity." Obviously "stupidity" is way too strong here, but the principle still applies.

In this case I would replace "stupidity" with "assumption". It was code-reviewed so that level assumed correctness. It's in OpenSSL, which is written by experts, so the millions of us that use it every day assumed correctness. Our customers know little about the workings of the internet, so they assume correctness.

This, incidentally, is the same sort of thinking that sometimes kills experts in dangerous situations: the assumption that someone smarter than us must by definition know more than us, so we can be lackadaisical. Witness Snow Fall (http://www.nytimes.com/projects/2012/snow-fall/?forceredirec...)

Re: 'Heartbleed' contributor denies he inserted it deliberately

#67

Post by ThursdayNext »

fit2rule wrote: post #22

I just can't understand why such critical components as OpenSSL just don't use Code Coverage tools like Coverity to find such things as this? Testing, coverage certification, static analysis: this would have been caught if these tools were being used.

This thread here:

http://openssl.6102.n7.nabble.com/Coverity-coverage-of-OpenS...

mentions that one OpenSSL developer used to see defect reports from Coverity (probably through Coverity's scan project). He states:

"Coverity used to, and perhaps still do, run scans of OpenSSL, which we had (have?) access to. I used to look at them and fix relevant ones, but got irritated with the false positive level in the end.

If Coverity were interested in fixing their bugs, I might get interested in looking at their reports again."

Of course this doesn't demonstrate that Coverity found this particular problem, and since he doesn't state what the false positive rate was it's difficult to know how reasonable his statement is.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#68

Post by DanBC »

danielweber wrote: post #50
talmand wrote: post #24

Earlier quoted context omitted.

Unfortunately the tech industry seems to be in mob mode recently so I would think the chilling effect is already well underway. The fact that it has been suggested this was done intentionally before they know the whole story suggests this to me. Never mind the fact that the code was apparently reviewed, so I guess the reviewer would have been in on it as well. There are always people who prefer to play the blame game…

Part of me wants to see the team give the industry a big middle finger and quit the project, and now companies relying on it have to start spending money to keep it up instead of expecting it to be provided for free. If something is important to you, you should spend more resources than zero on it.

Banks spend a lot of money on their login stuff. Some of those are really awful. Be careful what you wish for.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#69

Post by notacoward »

nuncanada wrote: post #61
orblivion wrote: post #59

Earlier quoted context omitted.

> Most static code analyzers could have caught this particular bug Does that mean nobody out there in the crypto world ran a static analyzer on OpenSSL in the last 2 years?

OpenSSL's code uses particularly bad MACRO's ifdefs that defeat static analyzer's capabilities to analyze the code...

I'm not sure how much those macros affect anything here. For one thing, most static analyzers are actually very good at cutting through that kind of crap, running its analyses on code that has already been pre-processed using exactly the same command-line flags as the real compiler saw.

Even if that weren't the case, I think this particular bug would still qualify as low-hanging fruit. It doesn't involve a lot of macros. It doesn't involve dynamically assigned function pointers. It's not limited to one execution through multiple iterations of a complex loop. I'm painfully familiar with the constructs that can defeat static analysis, and none of them seem present in this case. The code allocates a buffer of size N, then reads from an offset that's not checked to be less than N. That's kind of Static Analysis 101.

My biggest worry here, TBH, is that static analysis was done, and this was flagged, but it was buried among so many other reports - many of them false negatives - that nobody paid any attention. That would be truly sad.

Re: 'Heartbleed' contributor denies he inserted it deliberately

#70

Post by DanBC »

orblivion wrote: post #59
notacoward wrote: post #29

Maybe the silver lining here is that it puts the final nail in the coffin for "many eyes make all bugs shallow" - which was always total BS from the day it was uttered. There's so much code out there, much of it highly specialized and even project-specific, that there are very few eyes looking at any particular piece of code, and not all eyes are connected to the greatest of brains. Most static code analyzers could h…

> Most static code analyzers could have caught this particular bug Does that mean nobody out there in the crypto world ran a static analyzer on OpenSSL in the last 2 years?

... And wasn't the Debian bug caused by someone running a tool against the code and trusting the tool?
Post reply on HN