6 ms·

Wow. Was it really that simple? The heart beat request sends the text as well as the length it wants back?

by weavie 13y ago

danbruc 13y ago
Yes. They really only missed the check that the length of the data you sent and the length you said it is are the same.
- sp332 13y ago
  Well, they also failed to clear memory that includes sensitive data.
- pbhjpbhj 13y ago
  I'm not a programmer but .. why bother asking you what length it's supposed to be if you're going to strlen() it anyway? Sure you might need to allocate resources if this was carrying a proper payload but here we're just talking about an index. The spec not specifying the max length of the payload seems to be the problem??
  - danbruc 13y ago
    The maximum length is specified, 65535 bytes, because that is the largest number that fits into the length field. If you have a variable length field you always have to specify the length, otherwise you will never know the actual length and are for example unable to read the next field because you don't know where it starts. There is no real next field in the case of the heart beat packet, only padding, but you still have to figure out the length in order to know what to return. 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.
    - pbhjpbhj 13y ago
      Using UDP over IPv4 isn't the max payload size for a packet 65,507 bytes? That would mean that the max length that can be specified is already going to be a potential buffer overflow. What happens if you use specify a zero length? Seems that if the protocol were being defined loosely that should allow for "any length of field that fits within the transport packet" and so allow for larger datagram packets (a la Jumbogram). 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 ...
      - danbruc 13y ago
        TLS is usually used on top of TCP and therefore UDP limits do not apply. I have no idea how implementations on top of UDP or other datagram protocols deal with that issue. For a payload length of zero I would expect to just get back a payload of length zero, but I am be no means a network or even TLS expert and the standard may as well disallow this. And I am to lazy to look it up. 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.
y0ghur7_xxx 13y ago
Yes. Here is a PoC in Python: https://gist.github.com/takeshixx/10107280 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.
- smtddr 13y ago
  No, not "40".... "40 00". https://news.ycombinator.com/item?id=7555156 https://news.ycombinator.com/item?id=7555156 Such that "FF FF" will be 65535 (the 64kb everyone keeps talking about)
torrent-of-ions 13y ago
The bug is really that simple. The fact that you actually get back sensitive information in a high proportion of requests is due to a few other factors that are a little more difficult to explain. Either way it's a severe potential security breach that is directly attributed to this error.
masklinn 13y ago
> Wow. Was it really that simple? 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
- weavie 13y ago
  An excellent explanation. Thanks.
- gokhan 13y ago
  Haha. Your comment is so complex it needs another XKCD comic just to make sure all those [-1], (2), 1., [1]'s etc. don't get mixed up.
  - StavrosK 13y ago
    That's because it was the in-depth, technical explanation of the comic. Based on the GP's comment, the comic is exactly accurate.
ijk 13y ago
Pretty much, yes. Here's an analysis of the actual OpenSSL code: http://blog.existentialize.com/diagnosis-of-the-openssl-heartbleed-bug.html http://blog.existentialize.com/diagnosis-of-the-openssl-hear... Here's the patch itself: https://github.com/openssl/openssl/commit/96db9023b881d7cd9f379b0c154650d6c108e9a3#diff-2 https://github.com/openssl/openssl/commit/96db9023b881d7cd9f...

Wow. Was it really that simple? The heart beat request sends the text as well as the length it wants back?

by weavie 13y ago

Wow. Was it really that simple? The heart beat request sends the text as well as the length it wants back?

danbruc 13y ago

Yes. They really only missed the check that the length of the data you sent and the length you said it is are the same.

sp332 13y ago
Well, they also failed to clear memory that includes sensitive data.
pbhjpbhj 13y ago
I'm not a programmer but .. why bother asking you what length it's supposed to be if you're going to strlen() it anyway? Sure you might need to allocate resources if this was carrying a proper payload but here we're just talking about an index. The spec not specifying the max length of the payload seems to be the problem??
- danbruc 13y ago
  The maximum length is specified, 65535 bytes, because that is the largest number that fits into the length field. If you have a variable length field you always have to specify the length, otherwise you will never know the actual length and are for example unable to read the next field because you don't know where it starts. There is no real next field in the case of the heart beat packet, only padding, but you still have to figure out the length in order to know what to return. 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.
  - pbhjpbhj 13y ago
    Using UDP over IPv4 isn't the max payload size for a packet 65,507 bytes? That would mean that the max length that can be specified is already going to be a potential buffer overflow. What happens if you use specify a zero length? Seems that if the protocol were being defined loosely that should allow for "any length of field that fits within the transport packet" and so allow for larger datagram packets (a la Jumbogram). 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 ...
    - danbruc 13y ago
      TLS is usually used on top of TCP and therefore UDP limits do not apply. I have no idea how implementations on top of UDP or other datagram protocols deal with that issue. For a payload length of zero I would expect to just get back a payload of length zero, but I am be no means a network or even TLS expert and the standard may as well disallow this. And I am to lazy to look it up. 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.

y0ghur7_xxx 13y ago

Yes. Here is a PoC in Python: https://gist.github.com/takeshixx/10107280 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.

smtddr 13y ago
No, not "40".... "40 00". https://news.ycombinator.com/item?id=7555156 https://news.ycombinator.com/item?id=7555156 Such that "FF FF" will be 65535 (the 64kb everyone keeps talking about)

torrent-of-ions 13y ago

The bug is really that simple. The fact that you actually get back sensitive information in a high proportion of requests is due to a few other factors that are a little more difficult to explain. Either way it's a severe potential security breach that is directly attributed to this error.

masklinn 13y ago

> Wow. Was it really that simple? 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

weavie 13y ago
An excellent explanation. Thanks.
gokhan 13y ago
Haha. Your comment is so complex it needs another XKCD comic just to make sure all those [-1], (2), 1., [1]'s etc. don't get mixed up.
- StavrosK 13y ago
  That's because it was the in-depth, technical explanation of the comic. Based on the GP's comment, the comic is exactly accurate.

ijk 13y ago

Pretty much, yes. Here's an analysis of the actual OpenSSL code: http://blog.existentialize.com/diagnosis-of-the-openssl-heartbleed-bug.html http://blog.existentialize.com/diagnosis-of-the-openssl-hear... Here's the patch itself: https://github.com/openssl/openssl/commit/96db9023b881d7cd9f379b0c154650d6c108e9a3#diff-2 https://github.com/openssl/openssl/commit/96db9023b881d7cd9f...