3 ms·

All code is full of vulnerabilites. If you say your code isn't, then I'm sure it is. I just do the best I can to keep the error rate as low as possible. But it'

• saagarjha 6y ago
  It’s better to have automatic checks than rely on programmers being careful enough to remember to add them. For padding: this probably happens more on architectures that don’t do unaligned accesses very well.
  • btrask 6y ago
    Help me out here, because I'm really trying to understand. Are you saying that dangling pointers that blow up if you double-free them is an "automatic check"? If not, what kind of automatic check are you talking about? If the extra code is really that bothersome, just use a macro or wrapper function.
    • saagarjha 6y ago
      It's a much better situation than NULLing them out, because that hides bugs and makes tools like Address Sanitizer useless. A dangling pointer, when freed, will often throw an assert in your allocator; here's an example of how this looks like on my computer: $ clang -x c - #include <stdlib.h> int main(int argc, char **argv) { char *foo = malloc(10); free(foo); free(foo); } $ ./a.out a.out(14391,0x1024dfd40) malloc: *** error for object 0x11fe06a30: pointer being freed was not allocated a.out(14391,0x1024dfd40) malloc: *** set a breakpoint in malloc_error_break to debug Abort trap As you turn up your (automatic) checking this will be caught more and more often. Setting the pointer to NULL will silently hide the error as free(NULL) is a no-op and nothing will catch it. Thus, the suggestion here was 1. advocating adding additional code, which has historically been hard to actually do in practice, and 2. providing a suggestion that is generally worse.
      • btrask 6y ago
        Good points, thank you for explaining! I can see an argument for wrapping it in a macro so you can turn off nulling in debug builds (ASan might even have hooks so you can automate this, I know Valgrind does). But use-after-free is worse than just double-frees, and if you read a dangling pointer in production there's no real way to catch it AFAIK. Last I heard (admittedly been a few years since I checked), you're not supposed to deploy ASan builds because they actually increase the attack surface. So, your program's memory is full of these dangling pointers, and at some point you will have a bug you didn't catch and use one. And you can't even write an assertion to check that it's valid. What do you propose? And again to clarify, I'm not trying to advocate for hiding bugs. I want to catch them early (e.g. with assertions), but I also want to avoid reading garbage at runtime at all costs, because that's how programs get pwn'd.
        • saagarjha 6y ago
          > But use-after-free is worse than just double-frees From an exploitability point of view they are largely equivalent. As for the rest of your comment: my point of view is largely "you should catch these with Address Sanitizer in debug", so I don't usually write code like "I should assert if I marked this as freed by NULLing it out". If I actually need to check this for program logic, then of course I'll add something like this. The macro you suggest would alleviate my concerns, I suppose, and it wouldn't really be fair for me to shoot that solution down solely because I personally don't like these kinds of assertions in production. So it's not a bad option by any means, other than my top-level comment of this requiring extra code. I know some libraries like to take a pointer-to-a-pointer so they can NULL it out for you, so that is an option for your wrapper. And a double-free that doesn't crash can sometimes open up exploitable bugs too since it messes with program invariants that you didn't expect. But these are much rarer than the typical "attacker controlled uninitialized memory ended up where it shouldn't" so it's not a big deal.
          • btrask 6y ago
            Very reasonable! Thank you for the discussion :)