Max Rottenkolber max@mr.gy writes:
From what I understand about the bug (I have not seen the code) it sounds
like data length information
arrived both directly and indirectly in the client message and that a
conflict between them was not
scrutinized.
No. The bug was that the keep alive protocol in SSL mandates the server to echo arbitrary data back to the client. The bounds checks were wrong too, but at that stage it really doesn't matter. The design is just plain wrong.
I don't think you can say that it's _just_ the design is just plain wrong.
If I give you as specification:
- client sends a string S of length s. - client sends an offset o and a length l. - server sends back data l bytes taken from the address of the string plus o.
and ask you to implement it only using the CL package, you won't be able to implement it in any CL implementation using non-zero safety, and you won't be able to implement it in most CL implementations using (safety 0).
But those weren't the specifications, they are obviously bogus.
But assuming they were the following (still bogus, but rather reasonable specifications)::
- client sends a string S of length s. - client sends an offset o and a length l. - server sends back the substring of S starting at offset o, containing l characters.
This you could easily implement in CL, (as easily as in C), but again, while in C this is a heartbleed bug, in CL, it poses absolutely no security problem (unless you're using some certain implementations with (safety 0), which you should not have done anyways, you're really asking for problems, aren't you).
(defun heartbeat-data (S o l) (subseq S o (+ o l)))
(heartbeat-data "Hello" 0 64000) > Debug: Bad interval for sequence operation on "Hello" : start = 0, > end = 64000
So while the protocol didn't specify apparently what to do when (> (+ o l) (length S)), this would have been handled as any other generic protocol or server error, and no private data would be bled away.
http://cacm.acm.org/blogs/blog-cacm/173827-those-who-say-code-does-not-matte... http://jameso.be/2012/02/11/language-matters.html
So it's not just the specifications, it's the language implementations that are at fault here (not the ANSI C language, which clearly says that it's undefined to read an uninitialized array or outside of allocated memory, and therefore you could expect as with any CL implementation to have exceptions signaled in such occurences (since it's undefined, implementation could define implementation specific exception mechanisms)).
But the actual protocol specifications didn't even say that! They are actually quite reasonable, and this is clearly an implementation bug:
https://tools.ietf.org/html/rfc6520
The specifications of the protocol explicitely say:
If the payload_length of a received HeartbeatMessage is too large, the received HeartbeatMessage MUST be discarded silently.
and:
When a HeartbeatRequest message is received and sending a HeartbeatResponse is not prohibited as described elsewhere in this document, the receiver MUST send a corresponding HeartbeatResponse message carrying AN EXACT COPY OF THE PAYLOAD of the received HeartbeatRequest.