Here's my analysis of the vulnerable code:
static void vwr_read_rec_data_ethernet(wtap *wth, guint8 *data_ptr, guint8 *rec,
int rec_size, int IS_TX)
{
...
// [1] msd_length is signed!
gint16 msdu_length,actual_octets; /* octets in frame */
...
// [2] msdu_length is initialized with external data (from received packet)
m_ptr = &(rec[0]); /* point to the data block */
s_ptr = &(rec[rec_size - vwr->STATS_LEN]); /* point to the stats block */
msdu_length = pntohs(&s_ptr[vwr->OCTET_OFF]);
...
/* [3] sanity checking is done but because msdu_length is signed, values
such as -1 will pass the check */
if (msdu_length > (rec_size - (int)vwr->STATS_LEN)) {
msdu_length = (rec_size - (int)vwr->STATS_LEN);
}
...
/* [4] wrongly validated data is casted to size_t (unsigned int) type and
used as memcpy parameter potentially causing overflow of buffer
pointed by data_ptr */
memcpy(&data_ptr[bytes_written], m_ptr, msdu_length);
...
}
Mitigation:
Declare msdu_length as guint16 instead of glint16
