Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

The p pointer, along with payload and p1 contain data from a packet. The code allocates a buffer sufficient to contain payload bytes, with some overhead, and copies payload bytes starting at p1 into this buffer, and sends it to the client. Notably absent are any checks that payload actually indicates the correct size of the memory. Because an attacker can specify an arbitrary value for payload, she can cause this routine to read and return memory beyond the block allocated to p. In this case, the call to memcpy() would also violate ARR38-C. Guarantee that library functions do not form invalid pointers.

 

Compliant Solution (Heartbleed)

OpenSSL version 1.0.1g contains the following patch, which guarantees that payload is within a valid range. The range is limited by the size of the input record.

Code Block
bgColor#ccccff
langc
int dtls1_process_heartbeat(SSL *s) {          
  unsigned char *p = &s->s3->rrec.data[0], *pl;
  unsigned short hbtype;
  unsigned int payload;
  unsigned int padding = 16; /* Use minimum padding */

  /* ... More code ... */

  /* Read type and payload length first */
  if (1 + 2 + 16 > s->s3->rrec.length)
    return 0; /* silently discard */
  hbtype = *p++;
  n2s(p, payload);
  if (1 + 2 + payload + 16 > s->s3->rrec.length)
    return 0; /* silently discard per RFC 6520 */
  pl = p;

  /* ... More code ... */

  if (hbtype == TLS1_HB_REQUEST) {
    unsigned char *buffer, *bp;
    int r;

    /* Allocate memory for the response, size is 1 byte
     * message type, plus 2 bytes payload length, plus
     * payload, plus padding
     */
    buffer = OPENSSL_malloc(1 + 2 + payload + padding);
    bp = buffer;
    /* Enter response type, length and copy payload */
    *bp++ = TLS1_HB_RESPONSE;
    s2n(payload, bp);
    memcpy(bp, pl, payload);
    /* ... More code ... */
  }
  /* ... More code ... */
}

 

...