Versions Compared

Key

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

...

Despite the seriousness of the vulnerability, Heartbleed is the result of a common programming error and an apparent lack of awareness of secure coding principles. Following is the vulnerable code:

 


Code Block
bgColor#ffcccc
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 */

  /* Read type and payload length first */
  hbtype = *p++;
  n2s(p, payload);
  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 ... */
}

...

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, then copies payload bytes starting at p1 into this buffer and sends it to the client. Notably absent from this code are any checks that the payload integer variable extracted from the heartbeat packet corresponds to the size of the packet data. Because the client can specify an arbitrary value of payload, an attacker can cause the server to read and return the contents of memory beyond the end of the packet data, which violates INT04-C. Enforce limits on integer values originating from tainted sources. The resulting call to memcpy() can then copy the contents of memory past the end of the packet data and the packet itself, potentially exposing sensitive data to the attacker. This call to memcpy() violates ARR38-C. Guarantee that library functions do not form invalid pointers. A version of ARR38-C also appears in ISO/IEC TS 17961:2013, "Forming invalid pointers by library functions [libptr]." This rule would require a conforming analyzer to diagnose the Heartbleed vulnerability. 

 



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 ... */
}

Risk Assessment

Failing to enforce the limits on integer values can result in a denial-of-service attack, unauthorized disclosure of information, or to run arbitrary code.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

INT04-C

High

Probable

High

P6

L2

Automated Detection

Tool

Version

Checker

Description

Astrée
Include Page
Astrée_V
Astrée_V

Supported by taint analysis
CodeSonar
Include Page
CodeSonar_V
CodeSonar_V

IO.TAINT.SIZE
LANG.MEM.TBA
IO.TAINT.ADDR
IO.UT.HOST
IO.UT.PORT

(general)

Tainted allocation size
Tainted buffer access
Tainted network address
Untrusted Network Host
Untrusted Network Port

CodeSonar will track the tainted value, along with any limits applied to it, and flag any problems caused by underconstraint. Warnings of a wide range of classes may be triggered, including tainted allocation size, buffer overrun, and division by zero

Klocwork
Helix QAC

Include Page

Klocwork

Helix QAC_V

Klocwork

Helix QAC_V

SV.

DF2794, DF2804, DF2854, DF2859, DF2864, DF2894, DF2899, DF2904, DF2909, DF2914, DF2924, DF2944, DF2949, DF2954, DF2956, DF2959


Klocwork
Include Page
Klocwork_V
Klocwork_V
SV.TAINTED.ALLOC_SIZE
SV.TAINTED.BINOP
SV.TAINTED.CALL.BINOP
SV.TAINTED.CALL.INDEX_ACCESS
SV.TAINTED.CALL.LOOP_BOUND
SV.TAINTED.INDEX_ACCESS
SV.TAINTED.LOOP_BOUND
 

Parasoft C/C++test
9.5BD-SECURITY-INTOVERF, BD-SECURITY-OVERFRD, BD-SECURITY-OVERFWR Polyspace Bug Finder
Include Page
Parasoft_V
Parasoft_V

CERT_C-INT04-a
CERT_C-INT04-b
CERT_C-INT04-c

Protect against integer overflow/underflow from tainted data
Avoid buffer read overflow from tainted data
Avoid buffer write overflow from tainted data

Polyspace Bug Finder

Include Page
Polyspace Bug Finder_V
Polyspace Bug Finder_V

CERT C: Rec. INT04-C


Checks for:

  • Array access with tainted index
R2016a

Size argument to memory function is from an unsecure source

Size of the variable-length array (VLA) is from an unsecure source and may be zero, negative, or too large
  • Loop bounded with tainted value
  • Memory allocation with tainted size
  • Tainted size of variable length array

Loop controlled by a value from an unsecure source

Rec. partially supported.

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

Bibliography

[Seacord 2013]Chapter 5, "Integer Security"

 


...