Friday, January 13, 2012

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 }