Software flaw #3: CVE-2012-4298

CVE-2012-4298 is classical example of signedness vulnerability which I have explained in flaw #1. Affected software is Wireshark versions 1.8.x before 1.8.2.

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

Software flaw #2: integer overflow vulnerability

Following code illustrates integer overflow condition:

#include <stdio.h>
#include <memory.h>
#include <stdlib.h>

unsigned char *createTable(unsigned int w, unsigned int h, unsigned char *initialRow);

int main(int argc, char **argv)
{
    unsigned char row[2] = { 'a' };
    unsigned char *wholeTable;
    unsigned int user_provided_w = 0x400;
    unsigned int user_provided_h = 0x1000001;

    wholeTable = createTable(user_provided_w, user_provided_h, row);

    free(wholeTable);
    return 0;
}

unsigned char *createTable(unsigned int w, unsigned int h, unsigned char *initialRow)
{
    unsigned int n;
    int i;
    unsigned char *buf;

    n = w * h;
    buf = (char *)malloc(n);
    if(!buf)
        return NULL;

    for(i=0; i<h; ++i)
        memcpy(&buf[i*w], initialRow, w);

    return buf;
}
 
The purpose of createTable(...) function is to take width and height and an initial row and create table in which all rows are initialized with initialRow. However we can observe that there can be integer overflow condition (bolded line), when width and height will be big enough. Lets assume width = 0x400 and height = 0x1000001 in this situation n will be equal to 1024 (in decimal), so only 1024 bytes will be allocated. Following for loop will be iterated 0x1000001 times so heap buffer overflow will occur.

Software flaw #1: Signed conversion vulnerability

To better understand this kind of flaw let's consider following code:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>

int read_from_file()
{
    char size;
    char buf[32];
    buf[1] = 0;
    int fd;

    if((fd = open("fileSubverted", O_RDONLY)) == -1) {
        perror("[open file]");
        return -1;
    }

    if(read(fd, &size, sizeof(char)) == -1) {
        perror("[read size]");
        return -1;
    }
    if(size > 32) {
        fprintf(stderr, "Too big size\n");
        return -1;
    }

    unsigned char s = size;
    if(read(fd, buf, s) == -1) {
        perror("[read content]");
        return -1;
    }

    return 0;
}

int main()
{
    if(read_from_file() == -1)
        exit(1);

    return 0;
}
 

* the above code should work without an explicit casting of size variable to unsigned int type (the bolded line) but in my environment it doesn't - without this cast I get EDEFAULT (Bad address) error from read function.

Problem lies in implicit conversion of 'size' variable form char type to size_t (unsigned int) type, if user provides negative value for 'size' (let's say -2) it will pass the 'if(size > 32)' check and it will be converted to size_t type while invoking read(...) function which will be 0x000000fe in 2's complement representation. Provided that file is bigger than 32 bytes, 'buf' buffer overflow will occur.

Attack:

Commands below will generate file that will cause application to crash (Segmentation fault), due to buffer overflow which overwrites return address from read_from_file function:

echo -e '\xfe'`perl -e 'print "A"x250'` > maliciousFile

or in pure Bash:

echo -e '\xfe'`printf 'A%.0s' {1..255}` > maliciousFile

Countermeasures:

1) Best in this situation is to declare size variable as unsigned char

2) Change validation condition to: if( size < 0 || size > 32) { //generate error }