A remote code execution vulnerability in GNOME [LWN.net]

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 14:19 UTC (Tue) by leromarinvit (subscriber, #56850)
Parent article: A remote code execution vulnerability in GNOME

Cue sheet - now that's something I haven't heard in a very long time.

Letting parsers for a gazillion obscure formats loose on random untrusted inputs, without any user action, is a questionable idea IMHO. Doing that in an environment that isn't locked down to death, so that the most any exploit could do is generate a bogus thumbnail, is more than questionable I'd say.

to post comments

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 14:34 UTC (Tue) by madscientist (subscriber, #16861) [Link] (7 responses)

According to the comments at the end of the linked article, the parser IS run in a sandbox. Unfortunately there was an issue with that (unpublished as of yet) that was since fixed as well (according to the article).

So, yes, locked down. No, not locked down to death, apparently.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 15:13 UTC (Tue) by leromarinvit (subscriber, #56850) [Link]

I was about to reply to myself, after noticing that in the article. But that just goes to show that tightly sandboxing arbitrary code that wasn't specifically designed with that in mind is hard, and sometimes impossible.

I'm still not convinced the idea of automatically running parsers that evidently weren't designed to run in a completely locked down environment (if they were, the sandbox could be much tighter: e.g. only read and write on already open fds, and maybe brk with a low enough memory limit set via cgroups) on untrusted data is a good one.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 20:16 UTC (Tue) by rahulsundaram (subscriber, #21946) [Link] (5 responses)

> According to the comments at the end of the linked article, the parser IS run in a sandbox. Unfortunately there was an issue with that (unpublished as of yet) that was since fixed as well (according to the article).

Details of the sandbox changes itself is covered in https://blogs.gnome.org/carlosg/2023/10/10/on-cve-2023-43...

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 0:52 UTC (Wed) by roc (subscriber, #30627) [Link] (4 responses)

Having the main thread be fully privileged and running in the *same address space* as "sandboxed" threads is ridiculous and does not deserve to be called a sandbox at all.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 13:10 UTC (Wed) by fredrik (subscriber, #232) [Link] (3 responses)

Right, we should not blame seccomp for the failure to confine this vulnerability in tracker-miner.

I'm no C programmer, but would a better solution in this case be to use posix_spawn to start a separate process which solely reads the file being indexed and returned text. With the assumption that a separate spawned executable could be locked down stricter with seccomp than a forked thread in the same process namespace?

Still, the problem with the argument that seccomp only failed in this case because tracker-miner used it wrong, is that it is an argument of purity, like no true Scotsman...

On the other hand: OpenSSH also uses seccomp, and is implemented in C, and handles untrusted user input in a privileged environment. So, apparently for some true Scotsmen, like the OpenSSH developers, it seems to be possible to not fail to implement secure software in the C programming language. Though, that might be the exception which proves the rule?

The author of both the current fix and the previous implementation of seccomp in tracker-miners confirms (AFAIU) that the seccomp sandbox in tracker-miner was and is a compromise. Many of the libraries tracker-miner use to extract text from files were not designed with confinement and seccomp in mind. So it seems that tracker-miner had to lower the guards to get any useful output at all. Still, when that compromise was on the table, a better option would have been to abandon that aproach and find another more appropriate way forward. One that didn't compromise security for sake of features.

BTW, tracker-miner still use Gstreamer to parse files, despite Gstreamer being a known source of security issues. Reported by LWN already in 2016.
https://lwn.net/Articles/708196/

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 15:56 UTC (Wed) by NYKevin (subscriber, #129325) [Link]

> On the other hand: OpenSSH also uses seccomp, and is implemented in C, and handles untrusted user input in a privileged environment. So, apparently for some true Scotsmen, like the OpenSSH developers, it seems to be possible to not fail to implement secure software in the C programming language. Though, that might be the exception which proves the rule?

This is a poor example, because SSH has a functional requirement to offer remote access to an authenticated/authorized user. If you sandbox it to death, then it can't do its job, so you have no choice but to take a more permissive approach. On the other hand, the vast majority of software has no such functional requirement, and therefore can be sandboxed much more aggressively than SSH servers.

Remember: There's really no such thing as absolute security. There's "harder to hack" and "easier to hack." Sandboxing is, ideally, a defense-in-depth measure. Software should be designed to avoid failing or misbehaving on malicious inputs, regardless of whether a sandbox is present, but no software is perfect, so we sandbox things to further mitigate the risk.

A remote code execution vulnerability in GNOME

Posted Oct 12, 2023 10:54 UTC (Thu) by gfernandes (subscriber, #119910) [Link] (1 responses)

The OP did not blame seccomp.

The OP made a very valid point - running a thread in a sandbox, in the same process/address space where rhe main thread is NOT sandboxed, invalidates the sandbox.

A remote code execution vulnerability in GNOME

Posted Oct 12, 2023 13:52 UTC (Thu) by mcatanzaro (subscriber, #93033) [Link]

Indeed, that's the best TL;DR here.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 15:32 UTC (Tue) by fredrik (subscriber, #232) [Link] (19 responses)

It is notable that the target of this vulnerability, the file indexer tracker-miner, apparently had employed both ASLR and seccomp sandboxing in an attempt to prevent exploits.

That reminds me of the Chromium Rule of 2. Only in this case there was a sandbox, but the unsafe language still allowed untrustworthy inputs to compromise the system. Oh well, at least it was a mistake no real C programmer worth his salt would ever make, surely?

Granted, the project was started in 2005 when safer languages weren't such a hot topic as today, but can we at least stop and take notice now: The use of memory-unsafe languages are no longer appropriate when dealing with unsafe input! If at all.

</rant>

https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/security/rule-of-2.md

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 20:13 UTC (Tue) by rweikusat2 (subscriber, #117920) [Link] (17 responses)

The vulnerability is an integer overflow, ie, someone using atoi(3) to parse a number and assuming the result will always be positive. This an ancient UNIX interface which has been deficient (or unexpectedly featured) since its invention in the early 1970s (it's already documented as machine code subroutine for the first version of UNIX). The proper way to handle this is to use the proper interface for it (strtoul(3) --- standardized since the first ANSI C standard) and check for error returns.

I don't think the attitude behind this kind of "Just can't be arsed!"-coding can be fixed by doing it in a different programming language.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 20:36 UTC (Tue) by rahulsundaram (subscriber, #21946) [Link] (3 responses)

> I don't think the attitude behind this kind of "Just can't be arsed!"-coding can be fixed by doing it in a different programming language.

It may very well help. Languages like say Go or Rust have less legacy traps but also are typically used with optional linters like gosec and Clippy that warns about the common issues. I don't know whether C developers are usually doing this.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 20:57 UTC (Tue) by Wol (subscriber, #4433) [Link]

Well, when I set the coding standards for C programming, the rule was simple.

"I know we can't stop the compiler complaining, but you set the warning level to max and if you can't explain the warning away, it's a fatal error".

That was Microsoft C, and with warning level set to 4, we were calling a bought-in library and we couldn't suppress the "unused argument" warning. Anything else got fixed.

Cheers,
Wol

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 21:02 UTC (Tue) by proski (guest, #104) [Link] (1 responses)

I worked on C++ code in the past where we were making a great effort to catch possible issues in the code (~90% unit test coverage, code sanitizers, Valgrind, using multiple compilers with all warnings enabled), but I'm not sure we would catch that error unless testing the problematic value. With Rust, on the other hand, I'm confident that a non-exploitable panic is the worst thing we could get even without clippy.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 8:58 UTC (Wed) by NAR (subscriber, #1313) [Link]

The presence of atoi in itself is a code smell - and has been for the past 20 years.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 21:00 UTC (Tue) by dvrabel (subscriber, #9500) [Link] (11 responses)

    fn main() {
        let i = "4294903295".parse::<u32>().unwrap() as i32;
        let v = vec![1, 2];
        if i >= (v.len() as i32) {
            return;
        }
        /* SAFETY: Bounds already checked. */
        let a = unsafe {
            v.get_unchecked(i as usize)
        };
        println!("{} {}", i, a);
    }

This is broadly what the vulnerability is, implemented in rust. This segfaults and can presumably result in similar exploits in real code.

It took a bit more work and the unsafe block should make reviewers look more carefully but the comment and the (similarly incorrect) bounds check could easily lead the reviewer to conclude its ok, particularly if this was buried in a larger commit and was being reviewed on a Friday afternoon.

Rust is a better language, but does not negate the need for through unit, component, system, and fuzz testing.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 22:26 UTC (Tue) by walters (subscriber, #7396) [Link] (3 responses)

In Rust it's much more idiomatic to use iterators (as well as the checked `get()`) than direct array indexing because in many case it's actually *more* ergonomic, and doing so helps the compiler elide bounds checks actually.

For example https://doc.rust-lang.org/std/primitive.slice.html#method... is just way better than doing the equivalent indexing by hand.

So usage of `get_unchecked` is IME very unusual in Rust and would be a large code smell outside of very low level code.

A remote code execution vulnerability in GNOME

Posted Oct 10, 2023 23:49 UTC (Tue) by mathstuf (subscriber, #69389) [Link] (2 responses)

The `as` casts are also very non-idiomatic. I think as a translation it *works*, but yes, it's not what a "native" Rust programmer would write. Rather, someone that knows C, learned Rust-the-language but not Rust-with-its-stdlib.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 17:51 UTC (Wed) by NYKevin (subscriber, #129325) [Link] (1 responses)

I'm not even sure a person who knows C-but-not-Rust would put in those casts. I find it hard to believe that the average C programmer would choose to parse something into a u32 and then immediately cast it to an i32 - that just makes no logical sense at all. If you want a u32, you parse it into a u32. If you want an i32, you parse it into an i32.

OTOH, the second cast (v.len() as i32) looks like it's implementing the "usual integer promotions" rule from C (i.e. the rule that everything magically promotes to int if you so much as breathe on it), and the last one (i as usize) is pretty transparently "the compiler made me do it." But C programs do int promotion because it is implicit, not because C programmers consciously choose to promote everything to int. Would a C programmer really choose to write code that juggles three different types, if all of the casts have to be written out explicitly? I'm not very good with Rust, but even I would see that and think "there has to be a better way" - and in this case, it seems fairly obvious that you can just parse it as usize to begin with. Then all of the casts go away and the bug is also fixed.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 19:52 UTC (Wed) by ballombe (subscriber, #9523) [Link]

Compile your C code with g++ and suddenly you get warning for unsigned/signed mismatch.
That is what I do, and I only ever write C code.

This vulnerability is less about the C language than about the ISO C committee mental block about allowing explicit semantic qualifier for C type.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 0:55 UTC (Wed) by roc (subscriber, #30627) [Link] (4 responses)

No-one would write this Rust code unless they were malicious, it's totally unnatural.

If you're reviewing file parsing code and you see "unsafe", that's a huge red flag. Come back to it on Monday.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 13:18 UTC (Wed) by smoogen (subscriber, #97) [Link] (3 responses)

I could see it coded this way without 'direct' malicious intent:
1. It is a 'from another language' coder tasked with Rust code but not 'familiar' with what is Natural in Rust but familiar with their own language.
2. They are under a deadline and have been trying to deal with a time/speed issue in some code. They find this method is 'not natural' but faster. [Many sins in other languages occur because of this.]
3. They got this from some AI assist which found that comment and assumed it was good code.
4. They got this from reddit/etc where someone with direct malicious intent put it up in either a 'joke'(*) or to make something bad happen.

All of the items are poor coding in one way or another but not directly malicious.

(*) A long time ago I wrote a man page for a program which set real memory (rm) flag on files. This was to allow you to avoid getting swapped out in virtual memory on any file it was given. An additional bonus was to set the real fast flag on files so that the were set to nice -10 and get all the CPU. As a 'joke' it was sort of funny. When it was taken seriously... it was not.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 13:37 UTC (Wed) by Wol (subscriber, #4433) [Link]

Hmmm...

Many moons ago on a multi-user system, I set up a batch queue. Back in the day when an 800MB winchester drive was about 5" x 5" x 2' in size. Any jobs submitted to that queue were basically set to max cpu priority, max i/o priority. However, it didn't get used much because it was also set to max 30sec wall clock. Beyond that, any job got terminated with prejudice. It was, however, useful for getting little jobs done quick :-)

Cheers,
Wol

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 18:03 UTC (Wed) by mb (subscriber, #50428) [Link]

>1. It is a 'from another language' coder tasked with Rust code but not 'familiar' with what is Natural in Rust but familiar with their own language.

Well, such a person would probably never use 'unsafe' and probably also wouldn't use 'get_unchecked'. The person would probably just do 'v[i]'. Which is safe.

This code is totally not what happens in the real world.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 21:29 UTC (Wed) by roc (subscriber, #30627) [Link]

Those things theoretically could happen in but in practice they don't.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 14:07 UTC (Wed) by farnz (subscriber, #17727) [Link]

As a Rust reviewer, I pick on the following issues; let's assume that I don't spot the vulnerability, but that I throw this code back to you with the following comments. Can you rework to address my review comments, without making the bug more obvious?

I see three casts using "as"; these are dangerous because they don't do checks as part of the cast. Can you rewrite these using from, into, try_from or try_into?
The Rust compiler is great at optimizing out bounds checks in most situations; can you use get, instead of get_unchecked, and rely on the optimizer removing the bounds check? If not, why not?

Basically, you've got two things in there (get_unchecked, and as casting) which trigger my "this is likely to be buggy" radar. Add in "unsafe", which triggers my "this needs close inspection" radar, and I'm likely to spot that there is an issue when I try to get you to write better Rust.

A remote code execution vulnerability in GNOME

Posted Oct 12, 2023 9:04 UTC (Thu) by Tobu (subscriber, #24111) [Link]

I don't believe someone would write this innocently when the alternative is a simple v[s.parse::<usize>()?].

I looked at the cue-parsing crates I could find: cuna, rbchunk, rcue, cue_sheet, none of them use unsafe, rcue forbids unsafe_code. cuna does depend on parser library nom, which has uses of unsafe in six string splitting functions and has unsafe dependencies for memchr and float parsing.

I do wish clippy had a blanket lint against all uses of `as` for numeric casts, which are a code smell when from/try_from exist. Right now you have to go through this list, many of which are allowed by default, and there are holes.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 18:38 UTC (Wed) by adobriyan (guest, #30858) [Link]

strtoul() doesn't force to check for error. Programmer who is clueless to use atoi() will write strtoul() just as easily.
And there is no strtouint() so potential for clipping the result is still there.

A remote code execution vulnerability in GNOME

Posted Oct 11, 2023 0:53 UTC (Wed) by roc (subscriber, #30627) [Link]

It wasn't a proper sandbox at all, see my comment above.