
My heart is ok, but my eyes are bleeding - tptacek
http://blog.leafsr.com/2014/04/11/my-heart-is-ok-but-my-eyes-are-bleeding/
======
mindstab
"Naming a length variable ‘payload’ is borderline sociopathic"

~~~
TwoBit
I think it further indicates that not enough review of the code was done
before it was integrated. Dumb variable names like that should be caught at
review time.

~~~
bronson
I suppose the lack of interested reviewers is a decent indicator of how many
people/companies actually cared about the heartbeat feature.

------
PythonicAlpha
The simple statement that was given here:
[http://www.tedunangst.com/flak/post/analysis-of-openssl-
free...](http://www.tedunangst.com/flak/post/analysis-of-openssl-freelist-
reuse) that the developers just had to test with normal malloc to detect the
error is a little to simple for me. I can't see this "simple" solution.

Testing is utterly hard. It means that you need creativity, something that
many lack or are to lazy to. Many programmers are to lazy to think about
testing, I fear. That is the reason, there exist some very limited number of
really good software testers in the world.

When you want to find that bug in testing (taken, you even have the super-
malloc), you first have to come up with the idea, to send a falsified message
length for exactly this message type. Without this idea, even the super-hero-
malloc would have no chance at all to give any indication of a bug.

AND when you have such an idea, you would find the bug, even without any
super-hero-malloc ... any malloc or any freelist would suffice, because the
result would show you, that something bad is going on.

So the focus on the self-written allocator seems to get a little biased in
this discussion.

Yes, when writing security relevant libraries, you should be really, really
careful and you should take any chance you get to make it more save. And it
might have in some cases increased the chance of finding the bug _in
production_ , when just using the standard malloc (with security mitigation
active) -- but I really doubt, that it would have increased the chance in
testing.

I also must say, that really many successful software products use their own
specialized allocators -- the reason is simple: Because malloc is so
unspecific, it is not the fastest beast around -- and many specialized
allocators have their own security mitigation build in.

A last point: Google also wants (or wanted) to switch to OpenSSL. The reason:
Speed. So to blame the OpenSSL guys that they want to make their library
really fast, has a little bad taste. Everybody wants speed -- even those that
claim other _after_ something has happened. My thought for you: What would be
the benefit, if OpenSSL would fail because of bad speed, nobody would use it
but an other product -- and exactly that product would have the trouble we
found now in OpenSSL (or worse?) .....

Computer Industry can sometimes be unforgiving. How you do it, it is wrong.

~~~
akira2501
On some platforms, malloc(3) and free(3) are implemented in terms of mmap(2)
and munmap(2). Which means, if you go beyond your buffer space, you'll end up
in an unmapped page and cause a SIGSEGV or end up in NULL data.

Since they used the freelist/malloc wrappers, they avoided this trap.

Plus, IMO, if you're implementing a freelist, you're implementing 80% of what
a malloc is anyways. It's _mostly_ freelist handling, the only way to add
memory to a process aside from mmap(2) is setbrk(2). There's no way to give
memory back, so they only thing left for a malloc implementation to do is
create a freelist to reissue memory previously freed.

You can say you're still backed by malloc(3) if you want, but you're only
getting the setbrk(2) handling and throwing away everything else it's trying
to provide you.

~~~
PythonicAlpha
I don't understand what you want to tell me with all you said, after the
"Plus".

The point of self implemented allocators is not, that they are all better than
malloc or that the implementor is more wise or that there is some magical
additional stuff or even that some code is saved (the oposite is true) ...

The point is only, that malloc implements the most general case (what it must,
since it is the magical "I can do it all" tool (at least in memory
management)). When you go away from the general case you can find several
specific cases, that can be implemented specialized for this case -- and those
specific cases could be implemented very much more efficient (concerning
speed, not coding!).

That is the whole point. Also there are some specific allocators that also
implement some safeguides for common memory problems. But that seems to be
more of the exception, when I take into account the feedback.

~~~
akira2501
What I was trying to convey is that if you say "I'll just implement a better
freelist" you're really just fooling yourself. What you are doing is re-
implementing most of malloc on top of malloc, poorly.

If would have been better if they had gone whole-hog with a malloc library and
built it from the ground up for their specific purposes instead of half-assed
"freelist speedhacks."

------
julie1
He is right, after all, it is just a bug, like so many others some of them we
are not aware off, and it did not break internet. It just may leak
information, if and only if you really know your job in coding and are very
good. People that could be hired by organization with the money and the need
for it? Nooo.. You have to be paranoid to think so. And ... then throwing the
stone at openssl would be like distrusting all softwares: they all have bugs.
By the way, I never saw a software with a legal liability clause for bus for a
software... so ... if a user have a problem or money loss caused by softwares,
then he is the responsible for his choices.

------
cmbaus
There was some discussion here[1] that OpenSSL had implemented their own
allocators, but doesn't seem to be the case. As this article points out, a
quick review of the code will show that OPENSSL_Malloc() is simply malloc by
default[2].

[1]
[https://news.ycombinator.com/item?id=7565064](https://news.ycombinator.com/item?id=7565064)

[2]
[https://github.com/openssl/openssl/blob/a898936218bc279b5d7c...](https://github.com/openssl/openssl/blob/a898936218bc279b5d7cdf76d58a25e7a2d419cb/crypto/mem.c#L77)

------
lawnchair_larry
This analysis doesn't support the conclusion that the bug isn't severe at all.
And telling the reader that nobody cares about their email is rather smug. The
egos in the security community are out of control.

~~~
chrisrohlf
There have been remote code execution vulnerabilities in OpenSSL in the past,
yet no one panicked. Those vulnerabilities gave you what heartbleed does and
more. The worst part about heartbleed is that its easy to exploit and the
tools to do it are readily available. Its bad, as I state, but not the end of
the world. The main point of the post was to correct the public analysis of
the freelist implementation.

Please don't mistake a poor attempt at humor for ego.

------
aidenn0
I don't see a byline here; is this written by Chris?

~~~
chrisrohlf
Yes, it was.

