The blind leading the blind

Aristotle on 2007-06-06T16:16:07

This is a tale of an integer overflow vulnerability (paraphrased for the purposes of the tale, as are all following snippets):

dest = calloc(EXPR, sizeof(char));
/* where EXPR is a calculation that is never checked for overflow */

Of course, the maintainers of the software in question are studious and busy types and they would not let this go unfixed for long – they instituted a check to make sure that such a thing would never happen again:

int size;

size = EXPR;

if (size > INT_MAX || size <= 0) {
    return NULL;
}

dest = calloc(size, sizeof(char));

Marvelous! Now the code is safe…

Err, right. Undaunted and unphased by having made a mistake, they sagely nodded when they learned of it, and proceeded to implement a fix that would rid the world of this evil for good:

float size;

size = PARTIAL_EXPR_1;
size *= PARTIAL_EXPR_2;
size += PARTIAL_EXPR_3;

if (size > INT_MAX || size <= 0) {
    return NULL;
}

dest = calloc((int)size, sizeof(char));

And the world was a safer pl… err… I mean: and when they also got rid of floating point round-offs, the world became a safer pla…

OK, people, enough of the fairy tales. How do these people even come up with this stuff?!? Testing an integer for whether it’s greater than INT_MAX?! A float to calculate the length of a string!? I mean… I am… when I…

You can’t even parody this stuff.

This is bogosity as it belongs on DailyWTF, not into one of the most widely deployed codebases on the internet that is backed by several of the biggest corporations in IT. Please, please, someone tell me how PHP got to be as big as it is?

P.S.: oh yeah, and the current guard clause looks like this:

int charsize = sizeof(char);

/* still the same float-based calculation here */

if ((size > INT_MAX || size <= 0) || ((size * charsize) > INT_MAX || (size * charsize) <= 0)) {
    return NULL;
}

Portability is important, after all.


Agile development

speters on 2007-06-06T19:26:03

This is fun. It continues on...

This one looks like they found a few more overflows. Its all OK now.

Oops! Broke it again.

The only thing in C that

paranoidgeek on 2007-06-09T00:30:32

int charsize = sizeof(char);
The only* thing in C that they could assume .... and they didn't assume it ....
But really, this is just terrible. Where are all the decent developers ?

*well almost.

Re:The only thing in C that

Aristotle on 2007-06-09T00:58:38

Hell if I know. “Terrible” doesn’t even begin to describe it. If I hadn’t seen the CVS diffs with my own eyes, I would have thought it a joke. Nobody can be so comically incompetent, right? Turns out, yes they can be.