Memory Safety in Rust: A Case Study with C
willcrichton.net
willcrichton.net
Worse, over time as a codebase gets more complicated and older - and only if you're one of the few lucky enough to start with C code that isn't an unholy, inconsistent no-warnings disaster-mess slop bucket of horror - some of those warnings have a nasty habit of getting disabled so some middle manager can expedite some unreasonable commitment out a door to tick a box.
Call me a masochist but I enjoy writing C, particularly with valgrind, *san, afl, et al at my back. But I felt the point was well made by the author and I find it hard not to feel like a world where these problems simply didn't exist at all might be a bit nicer
What's hard about adding -fsanitize=address or -Weverything to your Makefile? Or running a program under Valgrind, or using the clang static analyzer or coverity? Using AFL can be a little cumbersome but still not that hard.
(it's worth looking at other sanitizers anyway, it seems not everyone is aware of their existence after all; if you can't put C away they're really great tools for retaining your sanity :))
Feel free to post your self-written applications (no fuzzers or similar with uninteresting attack surfaces) written in C for open review.
It's full of noise.
You can enable many others though, but -Weverything is virtually useless.
> gcc test.c
I get an error: test.c:21:3: warning: function returns address of local
variable [-Wreturn-local-addr]
return &vec;
After I fix that error, the program segfaults when running. Compile with asan: > gcc test.c -Fsanitize=address -lasan -g
Then we can start debugging these problems: ==3261== ERROR: AddressSanitizer: attempting double-free on 0x60040000dfd0:
...
#2 0x4008b5 in vec_free ./test.c:46
Not trying to say that this is the best workflow for debugging C, but the tooling does exist for these kinds of programming errors. > valgrind ./a.out
==4871== Invalid write of size 4
==4871== at 0x40073A: vec_push (test.c:40)
==4871== Invalid write of size 4
==4871== at 0x40073A: vec_push (test.c:40)
==4871== Invalid read of size 4
==4871== at 0x4007CD: main (test.c:55)
==4871== Invalid read of size 8
==4871== at 0x400769: vec_free (test.c:46)
==4871== Invalid free() / delete / delete[] / realloc()
==4871== at 0x4C2BDEC: free (...)
==4871== by 0x400770: vec_free (test.c:46) int new_capacity = vec->capacity * 2;
assert(new_capacity > vec->capacity);I found out about it through https://github.com/rohe/pysaml2/issues/451
But if you are a C programmer and speaking honestly, you need to have a better understanding of undefined behavior in C before you write any more code. Checking for signed overflow in C is extremely difficult to do correctly. This is a good starting point: https://blog.regehr.org/archives/213.
Not sure I understand why you think the assert() gets removed. Is this documented somewhere?
Hint: vec->capacity could be 0
Signed overflow (in int * 2) is an UB. If it happens, compiler is free to do anything. Eg. assume your assert can never happen and remove it.
Let me explain: Because signed overflow is an UB, compiler assumes that you would never write a code that overflows like that (because you write C, the assupmtion is that you are omnipotent), and knowing that, remove the assert that could possibly never ever happen, since it's a dead code anyway.
I've spend more than 6 years writting embedded software in C. God I love Rust.
I do not think its plausible that the compiler is emitting code which checks for an overflow and then in a deterministic fashion skips all dependent logic there after. Im sorry, but I think this line of argument has been taken to such an extreme it has detached from reality.
But all this is beside the point. In C, you may not invoke undefined behavior and then expect anything at all about the result. With malicious input, UB can and will result in RCE.
vec.capacity = 0;
Next, line 26: int new_capacity = vec->capacity * 2;
So we get new_capacity = 0 * 2, so new_capacity is 0. The assert: assert(new_capacity > vec->capacity);
Which translates to 0 > 0, it triggers a fail and we are done. If the compiler were to 'optimize' that out, then the compiler just introduced a very serious and fatal bug in our program, and we didn't even overflow or UB.So that assert works as intended, it forces new_capacity to grow until it hits INT_MAX or UB, at which point it will assert.
No, you don't see the point. Writing code in C that invokes undefined behavior is wrong, full stop. You seem to think that it's only wrong if you, the programmer, can imagine how it would go wrong.
> If the compiler were to 'optimize' that out, then the compiler just introduced a very serious and fatal bug in our program, and we didn't even overflow or UB.
Sure, in this case, the compiler can't optimize the assertion out. But it certainly can change the condition in the assertion so that it won't fire on overflow.
Once again, why would a compiler do that? To satisfy the Rust community's desire to spread FUD throughout the C community? (sorry to be so blunt, but im really scratching my head here)
A more logical thought process here is that signed operations aren't uniform across architectures and are sometimes emulated, leading to UB for certain edge cases, like overflow. Its more practical to believe that a future C standard will fix this loophole rather than think compilers will exploit this and punish their own community.
Very not impressed with this thread.
The theory is that constraint propagation enables optimizations that aren't possible otherwise. In particular, it lets the compiler avoid emitting code for conditions it judges are impossible to occur. This usually doesn't help much with C, but can be useful with C++, and many of the optimization decisions are driven by C++ performance.
To satisfy the Rust community's desire to spread FUD throughout the C community?
I'm a big fan of C, and consider it my primary language. I have barely used Rust and am not part of the Rust community.
A more logical thought process here is that signed operations aren't uniform across architectures and are sometimes emulated, leading to UB for certain edge cases, like overflow.
Personally, I would love if this was the vision for C that compiler writers held. But it is not, and has not been for at least a decade.
Its more practical to believe that a future C standard will fix this loophole rather than think compilers will exploit this and punish their own community.
Unfortunately there is no chance that this will happen. That ship has sailed. Compilers are taking ever greater advantage of undefined behavior for purposes of optimization. Overflowing a signed integer is undefined behavior, therefor real-world compilers can and do strip out checks that they determine would require undefined behavior to be occur.
I realize this is hard to believe, but try this article as an intro: https://blogs.msdn.microsoft.com/oldnewthing/20140627-00/?p=...
Or even better, just look at the assembly that is generated here:
#include <assert.h>
int add_one(int num) {
int increased = num + 1;
assert(increased > num);
return increased;
}
Both GCC and Clang completely remove the assert from the generated code: https://godbolt.org/g/5k43pV. To confirm, try adding "unsigned" to the "int" declarations and notice that it then keeps the check.I don't think either removes the check yet in the exact multiply by two case that you offer, but it is not safe to depend on this behavior.
Very not impressed with this thread.
I can see why you feel that way, but I think if you research this further you'll come to appreciate the stern warnings that people are giving you in this thread. C does not work the you want it to. Your choices are to adapt and avoid undefined behavior, or switch to a better language. I'm sticking with C because I think it's the best choice for what I do, but you need to be aware of the dangers if you are going to use it.
Sadly, a lot of people choose the third choice: write code in what they imagine C to be, fiddle with it until it compiles without warnings and, if they're trying extra hard, passes limited testing with Valgrind. Then they ship it.
If you wanted to check overflow condition, this assert doesn't help. If the capacity was almost MAX_INT, and you multiply it be 2, then compiler has freedom to do whatever, and not catch it because it must never be happening in the first place.
I suppose it could be worse: these sorts of examples could always use obfuscated C as their source for comparison.
- Return pointer to stack object (more common than you might think!)
- Incorrect size passed to malloc()
- Pointer to array outliving invalidation of array
Some of the remaining C code appears to be garbage intended to distract you from the errors people actually make, but there is some good stuff in there and I pull out similar examples when I'm trying to convince people not to learn C or C++ just to improve the quality of their greenfield projects.
Maybe when you're still learning how to program.
A combination of valgrind/sanitizers will catch all of these mistakes. The same class of mistakes can also be made in "memory-safe" languages, just replace pointer with index and memory with array.
There is a huge difference between out-of-bounds indexing leading to undefined behavior or an exception/panic.
int main() {
int x[16];
int y[16];
int z[16];
x[0] = 0;
y[0] = 0;
z[0] = 0;
y[18] = 3; // Valgrind thinks this is OK.
return 0;
}test.c:8:9: warning: array index 18 is past the end of the array (which contains 16 elements) [-Warray-bounds] y[18] = 3; // Valgrind thinks this is OK. ^ ~~
void foo(Foo *v) {
Operand blah;
/* Turns out that there's a use-list on some operands, and this adds &blah to that list in that case. */
copy_operand(&blah, &v->operands[1]);
free_foo(v);
}
> A combination of valgrind/sanitizers will catch all of these mistakes.They will catch only those mistakes that occur when you run them in tests. Plenty of memory safety CVEs still show up in programs that do aggressive fuzzing under valgrind/sanitizer testing. Or, as one aphorism has it, "testing cannot prove the absence of bugs, only their presence."
I used to think so, too. From John Carmack (https://twitter.com/ID_AA_Carmack/status/587077680652230656)
> Found two pointer-to-out-of-scope-stack bugs today. I like tight native code, but C/C++ still makes me worry a lot.
And then there's this dubious claim,
> A combination of valgrind/sanitizers will catch all of these mistakes.
Nope! 1) You have to execute the right code paths before Valgrind or any of the sanitizers will catch your use of a pointer to stack. In relatively simple cases, you might not catch the error even if you have 100% code coverage. 2) Not all platforms have Valgrind or sanitizers working on them, in fact, most don't.
> The same class of mistakes can also be made in "memory-safe" languages, just replace pointer with index and memory with array.
Sure, you could write an x86 interpreter in Java, and I'm sure that somebody has already done this. But these errors have much more severe consequences in C.
Another problem that can be largely avoided with a clear architecture.
1. The vast majority of buffers should be allocated at program startup, stored in a global variable, and freed at the end.
2. In almost all other cases, pointer ownership should just not be moved across functions (mostly "initializer functions" which return allocated memory). I think the idea that a function takes a data pointer without any idea how it was created, and then stores the pointer somewhere in its own structures, comes from garbage-collected languages and OOP culture which has an (I think) irrational aversion to globally architected dataflow. The messy object graphs that are created in this culture are just not manageable without a GC or another mechanically enforced discipline.
I'm not a fan of neither C++, nor Rust, nor garbage collected languages, for complex tasks. One can get away with less expertly architected programs in these languages. But, IMHO, they don't support, or do even impede, clear architecture, which ultimately leads to complexity. And this does mean hard to maintain and buggy software - it's just more memory-safe.
In my experience, good architecture does not materialize often enough to make this a viable strategy except for certain specialized teams. Consider writing code in C when you don't completely understand the problem and haven't devised a solution yet, or when you know it's going to get assigned to a junior developer and you don't have spare time for extra oversight. Or consider that you might ship poorly architected code because it's functional and shipping it will earn you money, or you might inherit large tracts of legacy code that are too expensive to refactor.
We all love ourselves some good architecture in our programs, but I'd rather have the penalty for bad architecture be "fails to compile" or "assertion error at runtime" rather than "our product has a security vulnerability". Just because the problem can be solved by throwing better, more experienced developers at it doesn't mean that we should. Let's throw the experienced developers at the more exciting problems and let our tools figure out memory safety for us... at least, most of the time.
> Legacy code is reduced to a fraction with good architecture, though.
I honestly don't understand this claim at all. I've only worked at one company where I didn't have to work with legacy code. The amount of legacy code I've been personally responsible for has varied from ~20 kloc (fairly manageable) to ~1 Mloc. I don't know what kind of good architecture could reduce that 1 Mloc. I ended up just throwing the address sanitizer at it, working on the test suite, and prioritizing refactors based on expected cost and payoff.
The thousands of C-based security vulnerabilities indicate that many experienced developers haven't figured out a "clear architecture" that prevents such issues.
Yes. Putting global state in a class and instantiating it once is pretty pointless. It's a lot of boilerplate and in the end just hides the intention. To the point that programmers confuse themselves about their own assumptions. Leads only to more bugs.
Furthermore, global state (as well as global constant data) enjoys the best support for memory management in existence: the linker / process loader. (well-tested, no maintenance overhead)
I'm not saying Rust isn't safer than C or not a sweet and useful language it is those things. What I am saying is comparing Rust to C without all the tooling that Modern C developers use is kind of disingenuous.
[1]: http://valgrind.org/docs/manual/manual-core.html#manual-core...
1. If using Int for indexing or any sort of len or count, make sure it's positive when needed, and within bounds of what's allocated. As in if you plan on allocating huge data, plan it out and use the right data type.
2. If you alloc, then free when done. If you free, set to Null; and before you free, check for Null.
3. If you realloc, in particular, check that it actually worked and prepare for basic error handling.
Rust requires all of these steps by default.
Finally, just test some of your code. Rust makes this easy, and encourages it.
I still really like C.
You can free(NULL), no problem. It will not do anything.
If this was found-in-the-wild C then I wouldn't be bothered with it, but this is completely contrived.
Most of beginners out there, who then cannot figure out the reason for their crashes, especially when it's not explicitly returned as the value of the function ;)
General programming experience helps a lot. Someone coming to C from a higher level language likely knows to read compiler output and warnings, use debugger, read diagnostic output. With modern C toolset, that experience will help solve 99% of such errors.
OTOH, someone coming to Rust encounters set of problems no amount of programming experience help to solve. IMO for beginners, Rust’s learning curve is dangerously close to that of pure functional languages.
Of course, this is not only an argument in favor of in Rust, but any memory-safe language. Rust just happens to address some of the same problem domains that C and C++ have traditionally been dominant in.
John Carmack, apparently: https://twitter.com/ID_AA_Carmack/status/587077680652230656
Edited to be explicit about programming language.
There is no point in measuring array sizes in size_t. I don't make bigger allocations >2G. (At some point this assumption will probably break and I will have to re-think my approach. Shouldn't we all move to 64-bit integers by default already?)
I think almost every Rust program I've ever published regularly encounters files greater than 2GB (even 4GB), and if I had used C and its `int` type everywhere, I'd be in for a very very bad time.
This of course doesn't mean I am allocating >2G on the heap, but I might memory map the file. Or there might be some other counter (line counter? byte counter?) where using `int` would just fail.
There are even some alternatives to my tools written in C, and either they or their dependencies use `int` instead of `size_t`, and that leads to actual bugs their end users hit in exactly the cases where files are greater than 2GB.
Getting integer sizes right is important, and it's not just in cases where you're putting >2G on the heap.
I prioritize on getting their values right :-). So far I have not encountered bugs due to my pretty uniform usage of int. But if they had to deal with allocations >2G, my programs would just die.
Yup, I make some exceptions as well, for example to measure time in microseconds, or to measure the size of very large streams. And of course I try to assert that all downcasts to int are valid, and that my integer operations don't overflow, etc. (why do CPUs still not support overflow exceptions?)
I'm pretty sure we could get rid of quite some historical baggage in terms of integer types. For example, I'm currently working on a network module, and there is a type socklen_t which is to indicate the size of a socket structure. I might be missing something, but to me there is no good reason not to use simply int.
Does that really make the protection against those mistakes less important?
No it doesn't. But from a C programmers point of view it's kinda like can you show examples of bugs in C that wouldn't immediately be found in code review or by tools like ASAN, Valgrind, Coverity or even just the C compiler, that Rust can solve. Those are the examples that would interest the C community.
Also the more complex the problem, the more likely it is incomparable between C and Rust. The prime example is a controversy whether Heartbleed would have happened in Rust. On one hand a direct C-to-Rust translation of the whole system with a custom memory-reusing allocator wouldn't have prevented the bug. OTOH such approach from Rust perspective is very contrived and unusable in practice, so one can argue nobody would have structured the code like that in the first place.
I'm just trying to clarify...it's lifetime safe, but not access safe, or leak safe?
Most people mean “safe Rust” when they say “Rust.”
Leaks are memory safe though. They’re hard, but not impossible, to get.
I disagree. I think they mean Rust code they see in use that has no explicit "unsafe" blocks. For example, most people are going to say "let mut v = Vec::<i32>::new(); v.push(0)" is "safe Rust." It is, in the sense that all the rules of Rust safety apply to the specific code quoted as-is. It also isn't, because 'Vec' is littered with unsafe.
If Rust never let you write code that could "go wrong", then it would not be useful for its target domain. Sometimes you need a "trust me" block where you call some internal allocator or trusted library function. It's more about the balance of affordances: is it easy to do the right thing in Rust, whereas in C it is easy to do whatever.
1. The (int PTR) cast of malloc() gives you away immediately. No cast in C.
2. > missing free on resize. When the resize occurs, we reassign vec->data without freeing the old data pointer, resulting in a memory leak.
Er... C has realloc() for that. Once again, you do delete() + new() only in C++, not in C.
BTW, you forgot other errors in your "C" program.