CVE-2009-3829: Wireshark Endace ERF Protocol Integer Underflow

Recently, this vulnerability was disclosed by Ryan Giobbi. The original bug report can be found here but unfortunately I get an “Access Denied” message when I attempt to view this link. So, the bug affects Wireshark prior to 1.2.2 release so here is the susceptible code as seen in 1.2.1…

extern int erf_open(wtap *wth, int *err, gchar **err_info _U_)
{
  int i, n, records_for_erf_check = RECORDS_FOR_ERF_CHECK;
  char *s;
  erf_timestamp_t prevts,ts; 
  erf_header_t header;
  guint32 mc_hdr;
  guint16 eth_hdr;
  guint32 packet_size;
  guint16 rlen,wlen;
  guint64 erf_ext_header;
  guint8 type;
  size_t r;
  gchar * buffer;
      ...
    rlen=g_ntohs(header.rlen);
    wlen=g_ntohs(header.wlen);
    packet_size = rlen - (guint32)sizeof(header);
      ...
    /* Read over MC or ETH subheader */
    switch(header.type & 0x7F) {
    case ERF_TYPE_MC_HDLC:
    case ERF_TYPE_MC_RAW:
    case ERF_TYPE_MC_ATM:
    case ERF_TYPE_MC_RAW_CHANNEL:
    case ERF_TYPE_MC_AAL5:
    case ERF_TYPE_MC_AAL2:
    case ERF_TYPE_COLOR_MC_HDLC_POS:
      if (file_read(&mc_hdr,1,sizeof(mc_hdr),wth->fh) != sizeof(mc_hdr)) {
	*err = file_error(wth->fh);
	return -1;
      }
      packet_size -= (guint32)sizeof(mc_hdr);
      break;
    case ERF_TYPE_ETH:
    case ERF_TYPE_COLOR_ETH:
    case ERF_TYPE_DSM_COLOR_ETH:
      if (file_read(&eth_hdr,1,sizeof(eth_hdr),wth->fh) != sizeof(eth_hdr)) {
	*err = file_error(wth->fh);
	return -1;
      }
      packet_size -= (guint32)sizeof(eth_hdr);
      break;
    default:
      break;
    }

    /* The file_seek function do not return an error if the end of file
       is reached whereas the record is truncated */
    buffer=g_malloc(packet_size);
    r = file_read(buffer, 1, packet_size, wth->fh);
    g_free(buffer);
     ...
  return 1;
}

This code resides in wiretap/erf.c file which contains the Endace ERF (Extensible Record Format) parser. The above routine opens up the ERF record, variable ‘packet_size’ which is an unsigned 32-bit long integer that is initialized with ‘rlen – sizeof(header)’ which as we can read from wiretap/erf.h is…

 /*
  * The timestamp is 64bit unsigned fixed point little-endian value with
  * 32 bits for second and 32 bits for fraction.
  */
typedef guint64 erf_timestamp_t;

typedef struct erf_record {
	erf_timestamp_t	ts;
	guint8		type;
	guint8		flags;
	guint16		rlen;
	guint16		lctr;
	guint16		wlen;
} erf_header_t;

However, a small value in ‘packet_size’ could result in an integer underflow during the ‘packet_size’ update in MC or ETH sub-header cases which you can read in erf_open() since they subtract ‘sizeof(eth_hdr)’ (which is 16 bits long) from the previously calculated ‘packet_size’. The subsequent allocation using g_malloc() could result in an attempt to allocate a huge portion of memory.
The fix for this vulnerability was the addition of a check for the maximum packet length before proceeding to the allocation like this:

     /* The file_seek function do not return an error if the end of file
        is reached whereas the record is truncated */
+    if (packet_size > WTAP_MAX_PACKET_SIZE) {
+      /*
+       * Probably a corrupt capture file; don't blow up trying
+       * to allocate space for an immensely-large packet.
+       */
+      return 0;
+    }
     buffer=g_malloc(packet_size);