Fernando J. Pereda’s blag

June 1, 2008

Security trivia III

Filed under: blag — Tags: , — Fernando J. Pereda @ 6:03 pm

People seem to like this kind of stuff, so here it goes. Take a look at this code:

#define MAX(a, b) ((a) > (b) ? (a) : (b))

struct st *dst, *src;
int size /* = some number of bytes you can control so that size >= 0 */;

/* Some code that sets up src and dst to valid buffers.
 * Everything but the last structure of src fits into dst. */

memcpy(dst, src, MAX(0, size - sizeof(struct st)));

Again:

  • What’s wrong with this code?
  • What environment do you need to exploit it?
  • Given such an environment, how can you exploit it?
  • How would you fix the code?

Remember, someone might have solved it in the comments. That could spoil the fun. You have been warned.

— ferdy

Advertisements

9 Comments »

  1. Looking at the man page for memcpy, the problem is the structure could overlap. The environment is you have an array of struct st’s and you’re removing one in the middle by copying the ones after it on top of the first. If the last one isn’t long enough, but greater than 0 then the copy will be attempted and it will overlap. I’d put the exploit code at the end of the array. And, to fix it, I’d change memcpy to memmove.

    I’m not a C coder, I’m just someone curious to know the answer.

    Brendan

    Comment by Brendan — June 1, 2008 @ 8:48 pm

  2. @Brendan: Well, dst and src point to valid buffers. That is, different memory areas which don’t overlap. memmove doesn’t fix this particular issue. Good try though, I considered adding a memcpy/memmove one because I know a particular project that used to get that terribly wrong.

    Try harder, or wait until someone finds it :P

    — ferdy

    Comment by Fernando J. Pereda — June 1, 2008 @ 9:21 pm

  3. well, the problem is that “size – sizeof(struct st)” will be converted to size_t. so, if sizeof(struct st) is bigger then size, max(…) will actually return some really huge value, and memcpy will overflow. i guess this is exploitable with every C implementation that does implicit conversions as they are specified by the standards (do you have some nice links where they are summarized in a decent manner?)

    as for fixing: casting sizeof(…) to int should do the trick (but maybe i miss something) – but i would just replace the whole construct with something more robust.

    Comment by mlangc — June 2, 2008 @ 12:17 am

  4. i think i understand now why the c++ template std::max(…) is not provided for different types…. considering what has been said above, this makes perfect sense now.

    Comment by mlangc — June 2, 2008 @ 12:25 am

  5. @mlangc: Exactly.

    Integer promotion rules require that ‘size – sizeof(struct st)’ be computed as an usigned type as per section 6.3.1.8 of the C99 standard.

    As for fixing it. There’s different ways to do it. I’d personally store the number of bytes to copy into a variable and do proper range checking there.

    I like this one because it is extremely subtle and difficult to see when reading code (it is easier in a 20-line snippet).

    — ferdy

    Comment by Fernando J. Pereda — June 2, 2008 @ 1:02 am

  6. Nice. 0 – 1 is BIG. I wouldn’t have gotten that as I didn’t know that sizeof returned an unsigned int. Thanks for the puzzle.

    Brendan

    Comment by Brendan — June 2, 2008 @ 2:04 am

  7. Very nice :-)

    Comment by Stian Skjelstad — June 2, 2008 @ 1:08 pm

  8. @Brendan: Yes, sizeof returns size_t which is guaranteed to be an unsiged type.

    — ferdy

    Comment by Fernando J. Pereda — June 2, 2008 @ 3:18 pm

  9. another problem might stem from the fact that structures are memory aligned and if you use a simple memcpy() to fill the structure you may overwrite padding space and the offsets (vars in the struct) do not point to the memory region you expect. example solution: use gcc attributes.

    Comment by Thomas — October 2, 2008 @ 9:03 am


RSS feed for comments on this post. TrackBack URI

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Blog at WordPress.com.

%d bloggers like this: