Xkcd: Heartbleed Explanation
xkcd.com
xkcd.com
Once I rephrased the final request as "Server, reply with the 500 letters of HAT", we finally had that light-bulb moment.
Your comment seems to imply an out of bounds access (read past the allocated buffer), but heartbleed has no out of bounds access.
Instead, it's a problem of malloc (and even more so openssl's freelist scheme) returning non-zeroed memory which can (and often does) hold previously allocated data combined with read(2) not overwriting the whole buffer and not checking read(2)'s return value, which means the aforementioned previously allocated data gets sent back.
http://security.stackexchange.com/questions/55343/how-to-exp...
It takes a special skill to eliminate all the non-essential information and produce something truly simple to understand, like the comic.
The only reason I ever manage to be short in text form is being able to go back and delete.
http://www.kevinandkell.com/2000/kk0730.html
You have the option to specify the length implicitly by using a terminator like a zero byte in a zero delimited string but this is sometimes a bad idea. First you have to scan the entire field just to figure out its length while with an explicitly stored length before the actual data you can just read the length and for example skip to the next field without ever looking at the field. Forgetting to properly terminate the field will of course get you in serious trouble, too. And last but not least using a terminator only works if you have an used symbol (or are willing to perform escaping). In the case of the heartbeat extension there are no restrictions on the content and you can put in arbitrary binary data and therefore something simple like a zero terminator does not work.
Is there a trend to leave definitions of these protocols as loose as possible (eg not specifying the response field in the heartbeat as a double/long or whatever) so as to allow hacking it to serve some other use later?
Guess I'm going to have to look at the DTLS protocol definitions ...
It is probably a really bad idea to make your protocol depend on details of an underlying protocol layer. Just imagine sending something over Ethernet and receiving it at the other end over WLAN where both protocols may disagree on the maximum packet size. You usually want your standard as precise as possible without imposing unnecessary and artificial constraints. Being allowed to stuff undefined amounts of data into a heart beat packet just to ease abusing the protocol really seems to call for trouble.
https://gist.github.com/takeshixx/10107280
On line 42 is the heartbeat request payload. 40 is the length of the message in hex = 64 bytes.
https://news.ycombinator.com/item?id=7555156
Such that "FF FF" will be 65535 (the 64kb everyone keeps talking about)
Close enough.
> The heart beat request sends the text as well as the length it wants back?
The heartbeat sends a payload prefixed by its size. That's perfectly normal design (for variable-size payloads), that way the handler reads the size, allocates a buffer[-1] and uses read(2) to read the payload into the buffer. Otherwise the handler would have to "guess" the payload size, and that never ends well.
The problem here is twofold:
1. read(2) may read less than requested, if an attacker gave a bigger size than the actual one for instance. That's why read(2) returns the number of bytes actually read
2. malloc(3) hands out a bunch of memory, without clearing it[0]. Depending on the exact allocator and application runtime, chances are this bit of memory is at least in part freed memory, which is filled with the content of previous allocations such as SSH keys or passwords or whatever
(2.) is compounded by OpenSSL having its own freelists on top of malloc which it does not clear, making it certain to hit previously allocated data
You're supposed to check the result of read(2) and adjust your payload size and only copy that to the output buffer. Or just error out if the sizes differ.
And ideally unless you have very specific reasons not to you'd want to use calloc(3), so that if you forget to check read(2) you return zeroed memory anyway. The first part was forgotten and the second one not done (because "needs fasts!"), the whole input buffer was copied in the output buffer and an attacker gets 64kb[1] worth of previous allocations data.
[-1] possibly adding its own constraints on top of that, here the payload's 64KiB so it's not relevant, in other contexts the server could refuse overly large payloads
[0] except on BSD with a malloc.conf using the J or Z options
[1] because the user-provided length is a 16 bit uint
e.g typing `man 2 read` into the shell on a Unix system will give you information about the read system call.
2's are for system calls, 3's are for members of the C standard library (usually).
If the input buffer was guaranteed to be large enough in this particular case I don't know but I can imagine an implementation that does not allocate the buffer for the 64 kiB worst case but just large enough to contain the actual request.
It was, since it used the same size to allocate the buffer and call read(2) it would never put more data than expected in the buffer.
> otherwise you can still hit whatever follows your input buffer.
Yes, but that's not the issue in heartbleed. My comment was about heartbleed, not about covering all the ways in which you can fuck up memory access in C.
> I can imagine an implementation that does not allocate the buffer for the 64 kiB worst case but just large enough to contain the actual request.
"just large enough" is impossible, you'll always over-allocate by at least 1 byte, and then to get the actual best precision you have to read the input data a byte at a time, performing a read(2) per byte. That's both slow and less readable.
You can do this. First allocate for and read the fixed length part, then determine the length of variable length part and finally allocate for and read the variable length part. This may of course still return less data then expected and leave you with uninitialized memory. And you may of course receive a larger buffer then you asked for.
Yes, but that's not the issue in heartbleed. My comment was about heartbleed [...]
Of course, I just wanted to say that zeroing memory may not be sufficient in the general case without bound checking because sometimes people have or get the impression that this would be a good and easy fix.
That's the part you can't do, you're reading data from a socket, you can't skip around with fseek(3), you read(2) or you recv(2) and if you don't store your data somewhere you lose it.
OpenSSL correctly reads the whole packet using the SSL record length. It then passes the packet off to the `tls1_process_heartbeat` function, which uses the second length field to do (variable names changed because the originals were terrible):
outgoing_packet = malloc(heartbeat_len)
memcpy(outgoing_packet, incoming_packet, heartbeat_len)
Edit: As a shortcut to establish my bona fides, I wrote a honeypot for this issue so I ought to know what I'm talking about:
Here's an analysis of the actual OpenSSL code:
http://blog.existentialize.com/diagnosis-of-the-openssl-hear...
Here's the patch itself:
https://github.com/openssl/openssl/commit/96db9023b881d7cd9f...
What advantage does returning the text give you?
This is, of course, glossing over the fact that TLS over a TCP link doesn't actually need a heartbeat. TLS/UDP does, granted. Ever heard of DTLS?
All I can think of is that the designer of the extension wanted to specify a mechanism with enough extensibility to allow requester-generated sequence numbers with arbitrary formats to be sent. Alternatively, if the sender makes the message unpredictable and compares the request and response messages and finds them the same, then this provides some weak "proof" that the responding server is the one that the request was sent to.
I don't find either of these particularly convincing though.
Allowing the request and response to carry a payload lets you mark each request with a unique identifier, so that you can recognise the resulting responses.
It's basically like an HTTP cookie. In reverse.
I have no idea why the payload is variable-sized, rather than being (say) a fixed 32 bits. I would guess the specifier didn't want to bake in assumptions about how much space a unique identifier would need.
Yep, although an up-to-64KB buffer seems like overkill you'd get a guy getting by on a byte, while the other one has a distributed system and wants UUID or something
EDIT: Nevermind, this thread [0] has more discussion about it.
For DTLS path MTU discovery (according to the heartbeat extension RFC).
It allows to build more abstractions in the client side without the server realizing it at all. For example:
* Setting as payload the time at which the packet was sent allows to calculate client-side the round-trip-time without adding complexity (you don't need to remember the sent time client side).
* Setting as payload an unique incremental ID allows an event-driven client that may have application side reordering of packets to understand the order of the arriving replies.
* Setting the payload to a client-known pattern is useful to detect server-side or network data corruption.
And so forth... ICMP ping-pong packets use the same mechanism.
We're all going to have to start reading more source code...