Systemd (PID1) crash with specially crafted D-Bus message
seclists.org
seclists.org
> the length of this is attacker controlled
This isn't the first time this has happened (https://www.openwall.com/lists/oss-security/2019/01/09/3). Uninformed opinion, but can we just like not stack allocate things, especially if their size is attacker-controlled? Linux has already moved away from VLAs for security reasons, I don't see why systemd shouldn't either.
> if we avoid stack allocation, then perhaps we need to instead allocate on the heap
That's what the fix does: https://github.com/systemd/systemd/commit/f519a19bcd5afe674a...
Speaking of limits, this is something that I think deserves more attention in education --- does the algorithm or code you create work for all inputs, especially those at the limits? Are you even able to determine those limits? Why does a path in D-Bus need to have a 32-bit length? Is 16 (64K) or even 8 (255) bits sufficient? If not, why? Can it be made sufficient?
Asking the important questions, learning, and understanding is ultimately going to be a far more useful than the current "just cut off the limb" attitude toward security that seems so prevalent today. Those questions should've all been asked and clearly answered at the design stage.
I'm not sure what path is in this context but if I assume file path (the rest of my post will look stupid if it's not) then 16 seems more than enough, but 8 has bitten many people in the windows world before. I'm also unsure of the context, is it always a path or is it a generic message that will be a path in this context? If it's the latter then there are many instances where 32-bit makes sense and then regardless of what is suitable for a path the 32-bit length is the contract.
do {
=> char prefix[pl+1];
Who do we git blame for this turd?
Why hasn't their static analysis tooling found this?I mean, he is a good programmer, but I’ve seen him (even in other projects) optimizing codes prematurely, which allows vulnerabilities to slip in. I understand the temptation, but many of his projects are mostly not performance critical. Not a good habit.
That typically doesn’t go well for long. The moment you write to that local variable (which technically need not happen, but if you don’t write to it, why have the variable at all?), you overwrite data in the heap and/or heap data structures. That will become a problem when the program reads that data or when the memory allocator walks the heap.
Debugging this kind of problem is hard, as the buggy code often will run fine. It ‘just’ triggers problems at some unspecified later time, likely in a completely unrelated part of the program.
So, guard pages ‘below’ the stack were added. They trigger exceptions that kill the offending program when it tries to write to or read from those pages. That doesn’t fully prevent this problem; large stack objects may jump over the protected area (especially in 32-bit code, you don’t want to give up lots of virtual address space to such guard pages, and the amount of address space ‘lost’ can add up fast in multi-threaded programs)
Another way to detect this is to periodically (say in an interrupt handler, or whenever a call to the memory allocator is made) check that the stack pointer has a valid value. That isn’t sure-fire, either. Firstly, it can only detect problems after the fact and secondly, it may not see short violations of the rules.
EDIT: maybe I got this. Basically, if my stack pointer jumps the protected page and ends up in the heap, I can address prefix[last_pos] without triggering any page fault, thus corrupting memory. Is this correct? I guess guard pages are a very weak protection mechanism, the OS should check the position of the stack pointer at every memory allocation.
- stack pointer is at S
- protected area starts at B, ends at E (with B<E<S)
- stack is decreased by something larger than (S-B), making it point somewhere before the start of the protected area
Compilers won’t allocate large local variables on the stack, so guard pages work fine for accidental stack overflows, such as when a call stack gets too deep. They also work reasonably well against intentional (by an attacker, not by the programmer who wrote the code) ones, but indeed aren’t perfect. That’s why some compilers have compiler flags that, when allocating a stack variable that they don’t know to be ‘small’ will insert extra code that will hit the protected area, even if the allocation jumps over it.
They don't generate stack probes? I'm used to this behaviour on Windows, where the compiler will generate code to explicitly touch the guard pages if it knows/suspects you may allocate more than a page:
https://geidav.wordpress.com/tag/stack-probing/
Yes, I know if you try to get it to allocate too much it will still crash, but that would be an "out of stack" error, so I'm a little surprised at that description.
https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.h...