I tested this exploit against an unpatched OpenBSD 6.6 machine and it works with the default mbox delivery, but not with maildir delivery (as hinted by the syspatch message). So if you use maildir delivery like me, you weren't exposed to this security hole. This is the sound of the world's quietest sigh of relief. I have some questions:
Is Qualys getting paid for this excellent work, and if so, by who?
Is there a plan to do a serious audit of execle related code in OpenBSD?
As a longtime OpenBSD user, I gotta say that OpenSMTPD is the part of the system I'm least comfortable with from a security standpoint. Too many rewrites, mulligans, CVEs. Very little of the web howtos match the official documentation because there's so much churn, which by itself is a red flag. And even without a logic bug, I'm surprised execle was used at all here. It was unnecessary and naive. I'll be honest, I'm in the middle of transitioning from qmail to OpenSMTPD, and this bug is making me consider notqmail.
>Is Qualys getting paid for this excellent work, and if so, by who?
I can't say I have any info on this subject, but having a group of people do stellar newsworthy offensive security research around widely used products is a good way to market your company for free and help drive sales for your security products.
This is such a clean and easy to read write-up on how the control flow led to the bug, and how it’s exploited.
Of course, that’s partly because it’s so damn easy to exploit. Here’s what an exploit email actually looks like;
$ nc 127.0.0.1 25
220 obsd66.example.org ESMTP OpenSMTPD
HELO professor.falken
250 obsd66.example.org Hello professor.falken [127.0.0.1], pleased to meet you
MAIL FROM:<;sleep 66;>
250 2.0.0 O.k
...
That executes “sleep 66” as root.
There simply must be a better way to parameterize calls to the MTA that contain remote/attacker provided input than exec’ing a shell. It should not all come down to being “absolutely sure” the input is escaped properly.
It’s interesting to think about the IF on Line 02.
It could have read “if (A)” and that would have been correct, but IMO even better to write out “if (A && !B)” in case anything changes with the surrounding code later.
The problem of course is that “A” and “B” are expensive functions which you don’t want to call twice, and it just is so annoyingly verbose to have to assign booleans and then compare those...
You almost want to be able to write out a truth table based on calling the functions and then handling their return values. A kind of 2D switch statement;
switch (A, B) {
case 1,1:
return 1;
case 1,0:
fix(B);
return 1;
default:
return 0;
}
Does any language support a multi-dimensional switch statement like this?
Hmm, maybe it’s not really any better that way either. The case statements are just not explicit enough.
Re: the language feature: This is standard in ML-family/typed functional languages (OCaml, Haskell...), and lately has been being adopted by a lot of newer languages, including Rust and Swift.
People give me funny looks when I tell them that I wish all languages had exhaustively-checked destructured pattern matching, but I really do think it’s one of the best “advanced” language features around!
While it might be annoying to use local variables, I usually find it's superior when debugging. It allows you to easily inspect the value and modify it before going forwards.
It also usually helps to make the code more self-documenting
It's definitely easier to deal with when using a debugger. But that brings a question to my mind: Why are there so many debuggers that make inspecting the results of intermediate expressions difficult? I should be able to step through my code at a finer granularity than a statement.
Re: self-documenting, certainly sometimes (I mean, you shouldn't play code golf...), but in this case the code in question was:
Well the messed up if logic is definitely a cause, i think part of the cause is the pattern of just validating input is safe instead of validating input at ingestion time and escaping just before output (shell out) time. I think doing both is a much more secure pattern*
*easy to sit here and armchair make claims of course. I've also only read this post and not the actual code so maybe there is reason for doing it how they did.
I'd add for writing the fix domain logic, probably most clear to fix it in one step, and then do the validation as a second step, instead of mixing the two. Minisculey less efficient but its much easier to follow if fixing and validating logic aren't mixed together.
Or it could pass argv directly to the execve() family instead of relying on the shell to process arguments. (They effectively do exec on sh -c 'cmd')
I am going to guess they will be smart enough to redesign this for the next major release, and the current patch is just that, a patch for the old design.
As pointed out in other responses, that's easily down with destructuring and matches in other languages, and even using a regular "case" by building the bits into a number (since the questions have boolean answers in this case).
However, as you yourself hinted, the cost (in C anyway) is giving up the short circuit - which might be expensive, or worse - might have unwanted side effects.
I'm not aware of a common language construct that would switch/match on both A and B, but defer executing B unless its value is actually needed.
It seems strange that people are blaming C for this. I see the real problem being that it is a unix pattern to use the shell to pass arguments to programs, even when that input is possibly malicious. Obviously doing this as root takes it from RCE to juggling with plutonium, but a non-confined non-root shell is pretty awful.
The code seems to go out of its way to avoid using the system() call to shell out, but then does exactly what system() would do.
Apropos of this, I was listening to the excellent On The Metal podcast, and Jonathan Blow made the point that Unix/shell has no type safety. Can we get a version of bash with strict types and typed arguments? (Does Powershell do anything in this regard?)
It's worth noting that this couldn't have happened to any mail server running on Plan 9, no matter how buggy it was.
Mail servers should run as nobody; mail box files are, in fact, world-writable, and their permissions should reflect that. Go ahead, critique the ergonomics of C's conditional expression syntax. But first, consider that this security model for a room full of terminals in the 1970s, where permission to accept connections on port 25 is also permission to format the hard disk, is totally nuts for a network-connected computer in the 2020s.
My main question is why isn't /bin/sh being executed with -r -- restricted mode? It seems weird that a safety-critical piece of code would just call out to /bin/sh without doing that, especially on openBSD?
This is a good question. Ideally (given the existing model) they would template an execv argv array, rather than a string to pass to sh(1), and execv() the MDA directly. It does not seem like the full generality of a shell is needed for pointing an smtpd at an MDA.
Of course, the reason they're invoking an external MDA is because this is classically how smptds and local mail delivery is separated. Is there a great reason for that? Not really. The MDA could be embedded in the smtpd.
Ouch. The root of the issue is that they do a validity check for the local and domain part of the recipient email address. If either one (or both) is invalid, they then check to see if the domain part is empty. If it is, they replace the empty domain with the default domain, and then say it's all valid, ignoring the fact that the local part might also be invalid.
It's interesting to see how these hacking-themed Hollywood movies have better reputations after two decades. When they were just released, responses from the hacking community were almost entirely "Hollywood is clueless". But after two decades, it becomes something different and gained additional historical values.
I dunno, I'm not convinced that's the case. It may be that there is some undercurrent of that, but I recall being super into WarGames in the 80s and very-very into it in the 90s. When I ran a dialup ISP, my mail servers were called wopr and norad... :P
Also, I think nerds just sometimes hate admitting they like a thing that is a caricature of themselves. I hate to admit it, but I really liked Hackers, and thought it had a lot of relevant nerd stuff going on it, ditto on The Matrix :)
I'm with you on Hackers. It's a guilty pleasure of mine and I still watch it from time to time. I remember watching it for the first time, scoffing at everything and thinking it wasnt even close to reality. But, it was fun. Also absolutely loved the soundtrack. I still listen to some of the artists to this day.
Idk, maybe having two decades of terrible hacker movies made people appreciate the old ones more. Personally I have always loved both Hackers and WarGames, but admittedly i wasn't around when they came out.
the PoC can be optimized: after way too much googling, I finally found a way to consume an arbitrary number of non-empty lines without using any special characters. ironically, it uses perl.
perl -00 -ne exit
unfortunately, the first line afterwards is also eaten. this is easily remedied by inserting one junk line though instead of a slide.
Can anyone provide some insight into possible motivations for why the authors may have chosen to use a shell here? I'm struggling to understand what value it adds over representing mda_command as an array of strings and execving that.
I don't mean this sarcastically; I'm genuinely curious about the motivations. The only thing I can come up with is that it's slightly more annoying to free an array of strings than it is to free a single string in C. Is that plausibly the only motivation to involve a shell here?
Wow, Openwall has been really at it when it comes to OpenBSD related vulns.
First with the user authentication vulns [0], now this.
For those running OpenBSD boxes: the patch is available through syspatch, but you may need to change /etc/updateurl to an official OpenBSD CDN, since the patch is still fresh and not yet distributed to all mirrors.
This isn't memory corruption, and you could easily end up with the same bug in Go (or Rust) code; the problem is that it's shelling out to subcommands to perform mail delivery and not screening addresses for metacharacters due to a logic bug.
In a better language than C, you could encode valid addresses in a proper type whose constructor would fail. But yes, you can treat everything as a string in any language.
You can also use type safety to enforce input validation in C. Just return an error code from the initializer, and annotate it must check. (Or, if you want to couple memory allocation with the checks, write a factory that returns null on input validation errors).
Various parts of the stdlib do this, though often without the compiler annotation. They often hide the struct members inside a compilation unit to prevent callers from bypassing the check (as much as is possible in a language with unsafe primitives).
C have types too. Put your string into a struct, do not expose struct fields directly but make an API to access/modify those fields and it won't be any different.
I did not say it was related to memory corruption. I said Go would be much safer than C (and it would be). When you remove an entire class of bugs, you can focus on other things... such as sound logic.
I love OpenBSD and use it a lot. But this notion of C everywhere for everything is wrong.
You also lose a lot of tooling that has been built up over the decades to help with C development, such as model checkers. Simply replacing one language for another doesn't help. It's silver bullet thinking.
This isn't an issue of language or platform, but an issue of process. The OpenBSD team has one of the best overall software development processes out there, but it is still very much a manual process. The class of exploits that are currently being discovered are often discovered via tool-assisted analysis. Such tools can also be used during the software development process to find errors that can be exploited. There are a dozen or so of such tools available for C currently ranging from OSS to commercial. Typically, these tools rely on model checking or bounded model checking, and they are quite effective at reducing attack surfaces or entire classes of errors beyond what most languages can provide on their own.
To take full advantage of model-guided development, however, requires a different software development process that is more difficult to adapt to older code bases overnight. Still, this adaptation is significantly less effort than, say, porting a C code base to Go or Rust. Model-guided development and other types of formal methods aren't silver bullets, and they aren't perfect. But, they are much more effective tools for building better systems than a language or compiler on its own.
I suspect that the OpenBSD developers will adapt to using a model-guided approach. They make use of some static analysis tools already. I find this much more likely than the OpenBSD team switching to Go or Rust, as it is a much more pragmatic solution and a more powerful solution.
Can you elaborate as to what "model-guided approach" OpenBSD is going to use?
I am skeptical of the claim that it is more "pragmatic" to adopt formal methods to achieve memory safety, as the number of projects that achieve memory safety through formal methods is multiple orders of magnitude smaller than the number of projects that achieve memory safety through just writing in a memory-safe language.
If you read the article, this RCE had nothing to do with memory safety. But, a model checker does a great job of finding memory issues as well as logic errors, if you use it correctly.
OpenBSD, AFAIC, has not announced any plans. But, there are plenty of tools out there that work. CBMC is a solid example and it is one I have used to great success with both systems programming and firmware development. As noted, this requires both a process change and a use of this tooling. Properly using assertions is a skill, but it can typically be cross-checked using existing manual processes with the added benefit of catching logic issues like the one that caused this RCE.
Clang's static analyzer and libfuzzer are fantastic, and the former has saved my bacon more than once. It also has lots of sanitizers (more than gcc) for run-time instrumentation, not that that would have helped in this case, but still.
That's great and all, but you'd create a host of new bugs unrelated to safety with a pile of new code in a largely unfamiliar language (for the openbsd devs, anyways).
Rewriting the world in some safe language isn't a panacea, this meme is getting very old.
The fact that memory-safe languages don't prevent every bug ever is not an argument against their use, any more than the fact that some people die in car accidents even when wearing a seatbelt is an argument that you shouldn't wear seatbelts.
Presumably, you could also build your C code with Address Sanitizer and use that in production, then most memory corruption would segfault. You'd have a performance hit of course, but could be worth it.
Is Qualys getting paid for this excellent work, and if so, by who?
Is there a plan to do a serious audit of execle related code in OpenBSD?
As a longtime OpenBSD user, I gotta say that OpenSMTPD is the part of the system I'm least comfortable with from a security standpoint. Too many rewrites, mulligans, CVEs. Very little of the web howtos match the official documentation because there's so much churn, which by itself is a red flag. And even without a logic bug, I'm surprised execle was used at all here. It was unnecessary and naive. I'll be honest, I'm in the middle of transitioning from qmail to OpenSMTPD, and this bug is making me consider notqmail.
This RCE is trivial and super bad.