[gnutls-devel] Incorrect SSL heartbeat bounds checking (not exploitable)
peter.dettman at bouncycastle.org
Fri Apr 11 11:02:38 CEST 2014
I concur with Frank, in that this code is not enforcing a minimum length
for the padding of 16 bytes as specified in RFC 6520.
if (len < 4)
The minimum packet length is 19, so that should be "len < 19".
if (hb_len > len - 3)
The payload is bounded by the minimum 19 bytes for non-payload data, so
this should be "hb_len > len - 19". I would wonder also whether that is
the most appropriate error code for a malformed message, but I don't
know enough about the available error codes to judge.
A further comment on the message format:
The "padding_length" is defined implicitly here i.e. "whatever is left
in the record following the previous fields" (subject to being at least
16 bytes). That's quite a strange choice; the only other example of such
a thing I can think of is ClientHello.extensions, where there is a more
apparent justification. Why not explicitly include the padding_length?
Perhaps more seriously, requiring that it be filled with "random padding
of at least 16 bytes" is 1) semantically pretty meaningless (later it
says that the receiver MUST ignore the contents anyway!), and 2)
suspicious in the light of DUAL_EC_DRBG and other RNG-related
revelations. I would advise to review what randomness is used by GnuTLS
here, and I suggest a deterministic construction of the TLS PRF keyed on
a hash of the payload (by analogy to RFC 6979) should be considered
instead of actual random data (especially if the same source is used for
On 11/04/2014 8:34 AM, Frank Li wrote:
> This may have already been discovered due to Heartbleed. gnutls's TLS
> heartbeat implementation is not vulnerable to heartbleed but also does
> not check the heartbeat length field matches the payload length. A
> heartbeat response may be larger than the heartbeat request. I bring
> this up for correctness and RFC6520-adherence sake.
More information about the Gnutls-devel