[pro] Heartbleed?

[Pascal J. Bourguignon]

Max Rottenkolber <max at 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-matter/fulltext
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.


-- 
__Pascal Bourguignon__
http://www.informatimago.com/
"Le mercure monte ?  C'est le moment d'acheter !"