5 ms·

The negative number is multiplied by 64, so it can then underflow and become positive again.

a3_nm 7y ago
Ah interesting... but then what could happen? Is the risk that the program could try allocating too much memory and crash? Calling this a "backdoor" seems excessive, no? -- at worst it's just a denial of service.
- bennofs 7y ago
  If you take (1 << 63) + 1 as size (which is a large negative int64), multiply that by 64 and you get 64. So it will call malloc(64) which will succeed. If other parts do the correct unsigned comparison then you have a heap overflow.
  - firebacon 7y ago
    This makes sense, but it also implies that there is no actual backdoor in TFA. The code as shown in the article is not exploitable without assuming more (actually exploitable) bugs somwhere else in the callsite (which the article doesn't mention). Or maybe we haven't figured out the actual vulnerability yet...
    - bennofs 7y ago
      Well, the rest of the code is also correct on its own though. If I tell the function to allocate X bytes, it should either fail or return a buffer that has space for X bytes. If the function returns less than X bytes if X happens to be a large unsigned int (that is a negative value if interpreted signed) then that is a bug and is exploitable.
      - firebacon 7y ago
        But that is not what happens in the code shown in TFA. Passing a large value into the method shown in the article will do nothing nefarious (assuming sizeof(size_t) >= sizeof(int)). It will either return a large allocation, or, more likely, fail because the amount of requested memory is too large. If you have a narrowing/casting bug somewhere else in your program, which BTW would produce an obvious warning, that would of course cause trouble as you have described. And while mixing signed and unsigned arithmetic for buffer sizes is, of course, a recipe for desaster, I think it's incorrect to claim that the allocatebufs method shown in TFA has a "backdoor" because of this. I feel that is a bit like saying memcpy has a backdoor because you might get your pointer arithmetic wrong when calling it.
- adisinom 7y ago
  It's hypothetical, but other code may expect a bigger buffer than was actually allocated. Suppose we're on a 32 bit platform and num is 0xF0000001. When multiplied by 64 (0x40), we'll end up allocating a 64 (0x40) byte buffer. But other code converting num to unsigned may be expecting a buffer large enough for 0xF0000001 64 byte records. After all, that was probably the reason for multiplying by 64.
- namirez 7y ago
  You would be right if this were an honest mistake, but a malicious actor can take advantage of this for all kinds of heap-based attacks. https://en.wikipedia.org/wiki/Heap_overflow https://en.wikipedia.org/wiki/Heap_overflow
- lokedhs 7y ago
  On Linux malloc doesn't crash if you give it a too large size. The cash only happens later once you're accessing a memory page that the kernel cannot allocate.
  - deathanatos 7y ago
    While malloc() won't "crash", it can and will, in some circumstances, return NULL. E.g., the following: printf("%p\n", malloc(0x7fffffffffffffff)); prints (nil) on my amd64 machine, as there is no way for the kernel to allocate that much virtual address space. (Even if it wouldn't back it w/ physical RAM.) The scenario being discussed in this subthread — having a negative number get inadvertently converted into a `size_t` and passed to `malloc()` is exactly the sort of way you end up getting a NULL back. But this is also not guaranteed.
firebacon 7y ago
But how does that result in a vulnerability? At best you can call the function with garbage input (negative number) and still receive a valid buffer from malloc, no? TFA calls this a "backdoor"; so how do you actually "get in" after you managed to get the backdoor through code review and deployed into production?
- PeterisP 7y ago
  You get a buffer that's too small for whatever data gets written there. That data (presumably controllable by the attacker) overwrites whatever other data is right after the allocated (too small) buffer; in many cases this can be leveraged to gain control of the program in ways that will depend the rest of the code around that backdoor. That code doesn't need to be buggy - simple, correct code that traverses e.g. a linked list can be abused to gain arbitrary memory writes and reads if you can overwrite the pointers in that linked list with values under your control; and these arbitrary memory writes can be abused to gain arbitrary code execution. Exploiting a heap overflow is not as straightforward as a stack overflow, but certainly possible, there are many real world code execution vulnerabilities that arise from a buffer overflow in heap.
  - deleted 7y ago
    [deleted]
  - firebacon 7y ago
    I don't think that what you are saying is correct. If you ask the method to allocate a negative number of bytes and the method returns a buffer which is greater than zero, that doesn't seem like a backdoor in the allocation method! Saying you can get a return buffer that is smaller than whatever amount of bytes you requested is wrong I think. Can you give an input to the second method that will result in a buffer smaller than the input value?
- 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).
thegeomaster 7y ago
It will become positive anyhow, since malloc takes a size_t as argument. My question still stands - how is the ability to coerce a program to malloc an arbitrarily large block a backdoor?
- saagarjha 7y ago
  The caller of allocatebufs will probably expect that allocatebufs returns num * 64 bytes, but due to overflow it could return fewer. Thus, it could end up reading or writing out of bounds.
mehrdadn 7y ago
Just a small nit: underflow is something else; this is still overflow (or wrap-around).
SAI_Peregrinus 7y ago
Signed overflow is undefined behavior in C.
- saagarjha 7y ago
  It is, but in this case the compiler will assume it never occurs and generate assembly that will function as the parent describes. Which, from a security perspective, is what matters in this case.

Ah interesting... but then what could happen? Is the risk that the program could try allocating too much memory and crash? Calling this a "backdoor" seems excessive, no? -- at worst it's just a denial of service.

bennofs 7y ago
If you take (1 << 63) + 1 as size (which is a large negative int64), multiply that by 64 and you get 64. So it will call malloc(64) which will succeed. If other parts do the correct unsigned comparison then you have a heap overflow.
- firebacon 7y ago
  This makes sense, but it also implies that there is no actual backdoor in TFA. The code as shown in the article is not exploitable without assuming more (actually exploitable) bugs somwhere else in the callsite (which the article doesn't mention). Or maybe we haven't figured out the actual vulnerability yet...
  - bennofs 7y ago
    Well, the rest of the code is also correct on its own though. If I tell the function to allocate X bytes, it should either fail or return a buffer that has space for X bytes. If the function returns less than X bytes if X happens to be a large unsigned int (that is a negative value if interpreted signed) then that is a bug and is exploitable.
    - firebacon 7y ago
      But that is not what happens in the code shown in TFA. Passing a large value into the method shown in the article will do nothing nefarious (assuming sizeof(size_t) >= sizeof(int)). It will either return a large allocation, or, more likely, fail because the amount of requested memory is too large. If you have a narrowing/casting bug somewhere else in your program, which BTW would produce an obvious warning, that would of course cause trouble as you have described. And while mixing signed and unsigned arithmetic for buffer sizes is, of course, a recipe for desaster, I think it's incorrect to claim that the allocatebufs method shown in TFA has a "backdoor" because of this. I feel that is a bit like saying memcpy has a backdoor because you might get your pointer arithmetic wrong when calling it.
adisinom 7y ago
It's hypothetical, but other code may expect a bigger buffer than was actually allocated. Suppose we're on a 32 bit platform and num is 0xF0000001. When multiplied by 64 (0x40), we'll end up allocating a 64 (0x40) byte buffer. But other code converting num to unsigned may be expecting a buffer large enough for 0xF0000001 64 byte records. After all, that was probably the reason for multiplying by 64.
namirez 7y ago
You would be right if this were an honest mistake, but a malicious actor can take advantage of this for all kinds of heap-based attacks. https://en.wikipedia.org/wiki/Heap_overflow https://en.wikipedia.org/wiki/Heap_overflow
lokedhs 7y ago
On Linux malloc doesn't crash if you give it a too large size. The cash only happens later once you're accessing a memory page that the kernel cannot allocate.
- deathanatos 7y ago
  While malloc() won't "crash", it can and will, in some circumstances, return NULL. E.g., the following: printf("%p\n", malloc(0x7fffffffffffffff)); prints (nil) on my amd64 machine, as there is no way for the kernel to allocate that much virtual address space. (Even if it wouldn't back it w/ physical RAM.) The scenario being discussed in this subthread — having a negative number get inadvertently converted into a `size_t` and passed to `malloc()` is exactly the sort of way you end up getting a NULL back. But this is also not guaranteed.

firebacon 7y ago

But how does that result in a vulnerability? At best you can call the function with garbage input (negative number) and still receive a valid buffer from malloc, no? TFA calls this a "backdoor"; so how do you actually "get in" after you managed to get the backdoor through code review and deployed into production?

PeterisP 7y ago
You get a buffer that's too small for whatever data gets written there. That data (presumably controllable by the attacker) overwrites whatever other data is right after the allocated (too small) buffer; in many cases this can be leveraged to gain control of the program in ways that will depend the rest of the code around that backdoor. That code doesn't need to be buggy - simple, correct code that traverses e.g. a linked list can be abused to gain arbitrary memory writes and reads if you can overwrite the pointers in that linked list with values under your control; and these arbitrary memory writes can be abused to gain arbitrary code execution. Exploiting a heap overflow is not as straightforward as a stack overflow, but certainly possible, there are many real world code execution vulnerabilities that arise from a buffer overflow in heap.
- deleted 7y ago
  [deleted]
- firebacon 7y ago
  I don't think that what you are saying is correct. If you ask the method to allocate a negative number of bytes and the method returns a buffer which is greater than zero, that doesn't seem like a backdoor in the allocation method! Saying you can get a return buffer that is smaller than whatever amount of bytes you requested is wrong I think. Can you give an input to the second method that will result in a buffer smaller than the input value?
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).

thegeomaster 7y ago

It will become positive anyhow, since malloc takes a size_t as argument. My question still stands - how is the ability to coerce a program to malloc an arbitrarily large block a backdoor?

saagarjha 7y ago
The caller of allocatebufs will probably expect that allocatebufs returns num * 64 bytes, but due to overflow it could return fewer. Thus, it could end up reading or writing out of bounds.

mehrdadn 7y ago

Just a small nit: underflow is something else; this is still overflow (or wrap-around).

SAI_Peregrinus 7y ago

Signed overflow is undefined behavior in C.

saagarjha 7y ago
It is, but in this case the compiler will assume it never occurs and generate assembly that will function as the parent describes. Which, from a security perspective, is what matters in this case.