3 ms·
This depends, obviously, on the code calling allocatebufs. The implication is that the calling code will most likely assume that a buffer of the right size (i.e
by nneonneo 7y ago
This depends, obviously, on the code calling allocatebufs. The implication is that the calling code will most likely assume that a buffer of the right size (i.e. "num" elements) has been allocated, while the real underlying buffer can be of any size depending on the low bits of "num".
For example, suppose the code was parsing user input as follows (a fairly common pattern):
unsigned int count = read_int();
struct buf *bufs = allocatebufs(count);
if(!bufs) goto fail;
for(unsigned int i=0; i<count; i++) {
bufs[i] = read_buf();
if(!bufs[i]) break;
}
This code isn't really safe because it passes an unsigned int to allocatebufs, but by default you won't see a warning for this. In the previous version of the code it would work fine - reject anything above 256. In the new "fixed" code, if count = 0x20000001 (for example) this will allocate 64 bytes and proceed to read up to 34 GB of data into the buffer. (A clever attacker can probably cause read_buf to fail early to avoid running off the end of the heap).
- saagarjha 7y agoEven if read_buf doesn't fail, an even more clever attacker may be able to still exploit a bug like this in a multithreaded environment: https://googleprojectzero.blogspot.com/2015/03/taming-wild-copy-parallel-thread.html https://googleprojectzero.blogspot.com/2015/03/taming-wild-c...
- firebacon 7y ago> This depends, obviously, on the code calling allocatebufs That was my point. The article claims to introduce a backdoor with the tiny change in the second example, i.e. "commit the change from the second example an you're in". But that just isn't true without assuming some other vulnerable code at the callsite. And arguably, the assumed bug is a bug of the assumed callsite and not a bug of the allocatebufs method! "See how easy it is to introduce a backdoor into C code which just one small change that looks completely harmless" might generally be true (debatable), but claiming that the change shown in the article (on it's own) is an example of this is incorrect and looks a bit like fearmongering.