Black Hat Amazon code question part 2

Thanks for the comments so far on the first day’s code question at Black Hat.

I’ll leave it a little while before posting the comments and answers, because it’ll give you a chance to think it through for yourself if you haven’t already done so.

Meanwhile, here’s the code example for day 2. What’s wrong with it?

wchar_t *fillString(
    wchar_t content, unsigned int repeat)
{
    wchar_t *buffer;
    size_t size;
    if (repeat > 0x7fffffffe)
        return 0;
    size = ( repeat + 1 ) * sizeof content;
    buffer = (wchar_t *) malloc ( size );
    if ( buffer == 0 )
        return 0;
    wmemset(buffer, content, repeat);
    buffer[ repeat ] = 0;
    return buffer;
}


.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: consolas, “Courier New”, courier, monospace;
background-color: #ffffff;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #006080; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }The language is C++, and as with the previous example, assume that everything that is not given above is perfect.

In case it is important, this was tested on an x86 system, although the flaw will also show up in x64. We were repeatedly asked that question.

4 Responses to Black Hat Amazon code question part 2

  • WS says:

    (Assuming LLP64 or LP64 aka int=4 bytes)

    “if (repeat > 0x7fffffffe)” is the problem (f overflow?:), and opens the door for overflow of size

  • Tom says:

    I’m not too familiar with C++, but this looks suspicious:

    buffer[ repeat ] = 0;

    What does the square bracket operator do for the buffer string? Set the value of the character at that position? If so then it will be setting some poor char in the middle of the string to 0. Probably it means to set the last one, in which case it should be using the size var in some fashion.

    That’s not a flaw. The line you quote is designed to set the last character to null, to properly terminate the string.

    Tuesday, August 31, 2010 16:17 PM by Alun Jones
  • interested says:

    Only thing I see is the memory allocated dynamically to buffer is never free’d, but the calling function would take care of this if its perfect. Is it something with dynamic memory?

  • Ooh, that’s sneaky.

    There’s one too many f’s in the initial comparison, so you’re comparing to 2^35-2 instead of 2^31-2. If repeat is 0xFFFFFFFF then repeat + 1 will overflow and size will be 0. The wmemset will then write a lot of data to a zero-length buffer.

    At first I didn’t see why this would be a problem on the x64 platform, but I see Microsoft Visual Studio defines int to be 32-bit even on x64 – is that usual? – so the addition would presumably still overflow before the multiplication with size_t changed the type to __int64.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>