Suppose you have the following code:
char *start, *end, *ref;
/* Some code that process user input so that start and end
* point to valid memory addresses and start + k < end.
* Where k is some constant you can't control.
* ref is made to point to some program-defined chunk,
* but you can't control this one.
*/
int n = end - start;
if (memcmp(ref, start, n) != 0) {
printf("Checksum mismatch. Access denied.\n");
exit(EXIT_FAILURE);
}
printf("Checksum matches. Access granted.\n");
This one is probably easier than the first one, but anyway:
- 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?
If you’ve seen me ranting about this in #exherbo-dev, you are not allowed to answer :)
— ferdy
I would have added a check against the length of ref in the code before the memcmp(). It can often be exploited since you can have a third outcome of the memcmp(), namely a segmentation fault, and then you can very fast brute first the secret password/token/checksum.
Comment by Stian Skjelstad — May 27, 2008 @ 12:11 am
Making memcmp segfault is a possibility, in certain situations, it’d mean a DoS. There’s something subtlier and worse in the code though.
— ferdy
Comment by Fernando J. Pereda — May 27, 2008 @ 12:13 am
We should ensure, that n is atleast as long as ref, (I assume we dont compare null terminated sequence, what is suggested by memcmp rather than strcmp). If we did not , user can give all possible sequences of length 1, namely 256, and gain access fast.
Comment by prak — May 27, 2008 @ 1:17 am
As required in the snippet, start and end are, at least, ‘k’ bytes away.
— ferdy
Comment by Fernando J. Pereda — May 27, 2008 @ 1:47 am
Sorry, understood this wrong. There is also possibility of integer overflow, for sufficiently large diffrence of (end – start), n would be negative, and memcmp would return 0. But I am still not sure if I understand your snippet…
Comment by prak — May 27, 2008 @ 2:31 am
Nah, that is even more stupid, it will be casted back to size_t by compiler, since memcmp has signature memcmp(void*, void*, size_t). That wont work, as I tought it will.
Comment by prak — May 27, 2008 @ 3:15 am
It’s a matter of getting an integer overflow on end – start so n is equal to zero. I can produce the desired results on an amd64 when end = start + 0xFFFFFFFF00000000.
Declaring n as size_t solves the problem.
Comment by Santiago M. Mola — May 27, 2008 @ 3:52 am
Well, 0xFFFFFFFF00000000 is just not doable. 0x100000000 is enough.
Comment by Santiago M. Mola — May 27, 2008 @ 4:06 am
That’s it. Pointer difference has type ptrdiff_t which is typedef’d to long on, at least, amd64 (it is to int on x86). You can ‘easily’ make n be 0 that way. The cast back to size_t wouldn’t be a problem since you already lost precision on the cast to int and n is already 0.
The solution here is to declare the variable as ptrdiff_t, even though size_t works, the solution is, in some sense, incorrect. (Probably size_t and ptrdiff_t are typedef’d to the same base type these days).
— ferdy
Comment by Fernando J. Pereda — May 27, 2008 @ 8:21 am
Nice one. :) Looks, I was close second time, but missed the obvious fact, that may sizeof(ptrdiff_t) > sizeof(int). Would you get any compiler warning with it?
Comment by prak — May 27, 2008 @ 9:15 am
I’m not getting any warnings here (on OSX), I shall try a newer gcc version on my exherbo installation later. I’m not sure it should emit a warning though.
Let’s see if I can get a better explanation of the kind of environment one needs:
* You need an architecture with 2-complement arithmetic (which would be virtually anything these days)
* You need, as some of you have already pointed out, that sizeof(ptrdiff_t) > sizeof(int), which would be any 64 bit architecture.
* An implementation that silently wraps around signed integer types (because we are overflowing a signed int). Linux and gcc work this way. I don’t know about other Unices, but I guess the BSDs are on the same track.
For references about all this in the standard (WG14/N1124), look for sections: 6.3.1.3 (paragraph 3), 6.5.6 (paragraph 9) and 6.5.16.1.
— ferdy
Comment by Fernando J. Pereda — May 27, 2008 @ 10:20 am
You need a difference of UINT_MAX+1. You need to introduce 4Gb of input data.
Comment by Santiago M. Mola — May 27, 2008 @ 12:02 pm
Just was able to test it: since gcc-4.3.0 (couldn’t check gcc-4.2.4) there is chance to be warned about it:
g++-4.3.0-alpha20080118 -Wconversion -fdiagnostics-show-option -o sec_triv sec_triv.cc
sec_triv.cc: In function ‘short int overflow_int()’:
sec_triv.cc:9: warning: conversion to ‘short int’ from ‘size_t’ may alter its value [-Wconversion]
I had to s/int/short int/ since I am on 32 bit machine.
Also -Wsign-conversion works; both are disabled by default in C++, and not included in -Wall, -Wextra. That would suggest add this one to my fav flags…
Comment by prak — May 27, 2008 @ 12:44 pm