7 ms·
am i the only one that thinks this is a total non-issue? if you're going to overflow a size_t, you're already in trouble. realloc is used for.. um, reallocatin
by foxhill 12y ago
am i the only one that thinks this is a total non-issue?
if you're going to overflow a size_t, you're already in trouble. realloc is used for.. um, reallocating data. it's probably grown many times before you come close to exhausting 32 bits of size (assuming i386). if you are coming close to 4gb allocations, you're already likely running out memory. as such, you're program is probably going to be killed for using too much memory before you are in any risk of overflowing size_t.
similarly, in a 64bit runtime.. well, you will certainly be killed by the kernel before you come close to overflowing size_t.
if you're reallocing based on untrusted user input.. well, again, you're already in trouble. "fixing" realloc will only make problems occur earlier (which i suppose is good).
EDIT: what i really should have said, is "make problems occur somewhere else".
finally, returning NULL for failure.. well, malloc doesn't fail. most (sane) codes don't check the return value of malloc because it doesn't make sense to. the kernel doesn't allocate memory until first access, at which point you're going to have trouble regardless.
colour me sceptical, but this seems like a fix to a non-existent problem.
- dezgeg 12y agoDefence in depth. This single overflow check could very well be the difference between denial-of-service-via-OOM and heap overflow. Should we then abandon ASLR or PIE or other defensive measures as well, just because some code is "already in trouble" due to a bad strcpy or something similar?
- foxhill 12y ago> This single overflow check could very well be the difference between denial-of-service-via-OOM and heap overflow OOM in general cannot be used for remote code execution. heap overflow can. > Should we then abandon ASLR or PIE or other defensive measures as well, just because some code is "already in trouble" due to a bad strcpy or something similar certainly not, ASLR is so general that it doesn't make sense to not do it. more importantly, they're entirely transparent to the programmer, and can be used on existing codes.
- dezgeg 12y ago> OOM in general cannot be used for remote code execution. heap overflow can. That was exactly my point. Using reallocarray() instead of malloc(x * sizeof(foo)) can prevent a heap overflow bug. For a concrete example (assuming 32-bit size_t): size_t user_input = ...; size_t* ptr = malloc(user_input * sizeof(size_t)); if (!ptr) return "FAIL: Out of memory"; for (size_t i = 0; i < user_input; i++) ptr[i] = whatever(); Now, a user_input of 2^30 + 1 overflows the multiplication so only 4 bytes is allocated, and then the for loop causes a heap overflow. Boom. But if the code was this: size_t user_input = ...; size_t* ptr = reallocarray(NULL, user_input, sizeof(size_t)); if (!ptr) return "FAIL: Out of memory"; for (size_t i = 0; i < user_input; i++) ptr[i] = whatever(); Now, a user_input of 2^30 + 1 would cause reallocarray() to return NULL, and the function returns. Heap overflow avoided by using reallocarray(). Of course, that code is still somewhat bad, since a user_input of 2^30 - 1 will cause the program to use large amounts for memory.
- foxhill 12y agoyes, i understand. as you correctly state, user input of 2^30 is.. well, it's unlikely. more likely, is a user being able cause a program to do billions of reallocs. in which case, OOM is going to happen before overflow and exploitation.
- deleted 12y ago[deleted]
- glass- 12y ago> am i the only one that thinks this is a total non-issue? OpenBSD's slogan boasts that there have only ever been "two remote holes in the default install", one of those holes would have been prevented by this.
- foxhill 12y agowhilst i appreciate the prudence of this measure, it doesn't do anything for existing programs. this will not fix any codes in OpenBSD that currently ship with it.
- tedunangst 12y agoIf only there were some way to modify those programs to use this function...
- foxhill 12y agoeither you mean intercepting malloc/realloc calls, in which case can't happen as the function signature is different, or you mean literally modifying the source code of existing programs, and releasing that. which, lets face it, in the BSD is space (and particularly with regard to memory allocations from user input), is highly unlikely to change any time soon.
- clarry 12y ago> or you mean literally modifying the source code of existing programs, and releasing that. > > which, lets face it, in the BSD is space (and particularly with regard to memory allocations from user input), is highly unlikely to change any time soon. I'm not sure how to parse your sentence. Are you saying OpenBSD (which should be known for its mass audits) isn't likely to make changes in its source code any soon? http://freshbsd.org/search?q=reallocarray http://freshbsd.org/search?q=reallocarray
- meowface 12y ago>"fixing" realloc will only make problems occur earlier (which i suppose is good). Yes, you should certainly suppose it's good, because it means the difference between an early out-of-memory error and a serious security vulnerability that can result in arbitrary code execution, like a buffer overflow. That is the point of wrappers like these: to improve security. >if you're reallocing based on untrusted user input.. well, again, you're already in trouble. You are in trouble, but the trouble will be considerably less severe if the untrusted input hits this function instead of hitting malloc or realloc.
- foxhill 12y ago> Yes, you should certainly suppose it's good, because it means the difference between an early out-of-memory error and a serious security vulnerability that can result in arbitrary code execution, like a buffer overflow. my point is that this scenario is so improbable that it's not worth worrying about. > You are in trouble, but the trouble will be considerably less severe if the untrusted input hits this function instead of hitting malloc or realloc heartbleed, SQL injection, and hundreds of other vulnerabilities are more likely to occur from untrusted user input. if you overflow a realloc, you don't know where the destination address is. you're far away from the stack, too. there is very little (if any) risk. i have never come across a realloc based exploit.
- tedunangst 12y agoYou've never heard of the one openssh exploit?
- foxhill 12y agoprior to now, no. if you're referring to the 2003 exploit, well, i was 14 when that happened. as i previously mentioned, an exploit from allocating data based on unvalidated untrusted user input is.. practically a tautology.
- tedunangst 12y ago
- scott_s 12y agoImagine that this allocation is for a buffer that is supposed to take user input. If the size is too small, you could easily be exposed to the classic buffer-overflow style of attacks. (That is, users know it's possible, craft a scenario to induce it, then deliver payloads.) So, yes, it's an issue.
- foxhill 12y agowell the solution is to validate user input. user input should certainly be limited to 4gb, at least. a wrapper around an allocator is definitely not the solution.
- scott_s 12y agoIf you're going to validate user input, you first need to get it into kernel space, which requires allocating memory for it. User input is not limited to 4 GB. Users can provide whatever they want; that's the point. You have to code for malicious users who are pushing your system towards its edge cases, trying to find holes to break through.
- foxhill 12y ago> If you're going to validate user input, you first need to get it into kernel space, which requires allocating memory for it. i don't buy this - if you are going to allocate space, you must know the size of the space you are allocating. which you can validate, and certainly does not require going into kernel space to do.
- clarry 12y ago> well the solution is to validate user input Oh come on, what do you think the purpose of reallocarray is? It's exactly to include that boilerplate for you so that you don't have write your own "validation" before every alloc.
- Dylan16807 12y agoThe user can still consume 99.9% of your address space. It is not a substitute for validation.
- Sanddancer 12y agoThe Linux kernel doesn't allocate memory until first access. Most other Unixes actually allocate when you ask them to allocate. So on BSDs, etc, malloc can and will fail when you ask the computer to allocate more memory than it can actually access. Overcommitting RAM is a rather silly "optimization" and tends to cause more trouble than it actually solves.
- foxhill 12y agodarwin, NT, and linux all do this. that is the majority of systems. it's not a silly optimisation at all. at worst, it performs exactly as well as malloc. at best, it saves allocating memory. which in the mobile platform (again, most of computing), is a big deal.
- Dylan16807 12y agoIt's also rather silly to need 50GB of never-used swap space to back up a fork-heavy program.
- kazinator 12y agoWhether or not mmap and malloc nicely return null, or whether you get a segfault on first access, depends on what OS you're using and how it is configured. You can configure Linux not to ovecommit memory. I don't think it's good practice to write user space code on the assumption that "modern OS's overcommit anyway, so we will never see null bubble out of malloc". There are conditions where that may be justified, but one example where that doesn't fly is writing portable, reusable components.
- wahern 12y ago1) malloc absolutely returns NULL on Linux, even with overcommit. Using setrlimit (which properly sandboxed daemons should be doing), it's trivial to get malloc to return NULL. 2) The issue isn't whether malloc returns NULL. Technically speaking, unsigned arithmetic doesn't even "overflow", which in C is a term meaning the result is undefined. It returns the value modulo 2^N. In other words, in the vast majority of situations malloc will succeed, but return a buffer smaller than you requested. 3) According to your logic, people shouldn't check object boundaries, either. Many buffer overflow exploits utilize arithmetic overflow, because usually the memory you're interested is nowhere near the object you're overflowing. And we know these overflows happen and can be triggered even when the application isn't using very much memory. People with your opinions shouldn't be allowed anywhere near C code.