LPE and RCE in OpenBSD OpenSMTPD(openwall.com) |
LPE and RCE in OpenBSD OpenSMTPD(openwall.com) |
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.
It is pretty ridiculous just how trivial and severe this is, indeed.
However, given the lower prevalence of OpenBSD, I'd be interested to know whether anyone has any data on whether this is being exploited in the wild.
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.
It's only free from external cost though. The time for the staff at Qualys is (probably) not free. ;)
See:
https://en.wikipedia.org/wiki/Qualys
My guess is they use OpenBSD and this is part of 'give back'.
https://man.openbsd.org/syspatch
https://www.openbsd.org/errata66.html#p018_smtpd_tls
https://www.openbsd.org/errata66.html#p019_smtpd_exec
There is also a new portable release of OpenSMTPd - 6.6.2p1: https://www.mail-archive.com/misc@opensmtpd.org/msg04850.htm...
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.
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.
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.
https://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-...
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.
Also, it's a bashism. It's not implemented by the OpenBSD's /bin/sh.
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 :)
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.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?
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.
It's /etc/installurl not /etc/updateurl.
I love OpenBSD and use it a lot. But this notion of C everywhere for everything is wrong.
01 if (!A || !B) {
02 if (!B) {
03 fix(B);
04 return 1;
05 }
06 return 0;
07 }
08 return 1;
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.
It also usually helps to make the code more self-documenting
save_A = A;
save_B = B;
if (!A)
fix(A);
if (!B)
fix(B);
return (some_expression of save_A and save_B);match (a, b) with | (1, 1) -> 1 | (1, 0) -> (fix b); 1 | _ -> 0
*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.
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.
if (A && B) {
return 1
else if (A && !B) {
fix(B)
return 1
else {
return 0
} 553 5.1.0 Sender address syntax error(So, at a minimum, it's a local user privilege escalation, of any user to root, in the default install?)
switch (!!a) & ((!!b) < 1) { case 3: return 1; case 1: fix_b(); return 1; default: return 0; }
If you rewrite 3 as “A & B”, this turns into feature flags, and is less crazy looking (not sure arithmetic is allowed in case statements though).
Better (regardless of language) is flow like this:
if (!b) fix_b(); return a && b;
Re: self-documenting, certainly sometimes (I mean, you shouldn't play code golf...), but in this case the code in question was:
2218 if (!valid_localpart(maddr->user) ||
2219 !valid_domainpart(maddr->domain)) {
....
2234 return (0);
2235 }
2236
2237 return (1);
It's not obvious to me what you would name the intermediate booleans that would be more self-documenting than the function names used there.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.
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).
fn main() {
for i in 1..102 {
match (i % 3 == 0, i % 5 == 0) {
(true, true) => println!("FizzBuzz"),
(true, false) => println!("Fizz"),
(false, true) => println!("Buzz"),
(false, false) => println!("{}", i)
}
}
}
Not only is it obviously correct, but if you somehow manage to forget a case the compiler will tell you. {
if A && B return println("FizzBuss");
if A && !B return println("Fizz");
if !A && B return println("Buzz");
if !A && !B return println("{}", i);
}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.
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.
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.
Which tools would I start googling if I wanted to learn more about this?
Rewriting the world in some safe language isn't a panacea, this meme is getting very old.
The libraries people write to prevent bugs like these from happening (at a low level, in the "keep metacharacters out of the command line" sense, not in the "don't accidentally put a conditional in the wrong place" sense which is the proximate cause of this bug) work in most languages, probably even C. By way of example: Ruby has a solid command line wrapping gem.
I think "you could use type checking to break that logic bug" shares a lot of structure as an argument to "the key to security is input validation". It's true-ish at a superficial level but almost totally useless in practice, because it depends on foreseeing what the exploitable bugs will be. If your proposed defense relies on a catalog of possible bugs, it's not a real defense.
Solid, ambitious type systems are a good thing. For other reasons.
I know that's your hobby horse, and I said "theoretically" specifically to head off this argument. I am also skeptical that type systems will prevent logic bugs like this in practice. I'm skeptical of formal methods for the same reason. (Of course, type systems are just simple formal methods.)
Decomposing logic using Hoare triplets and properly incorporating assertions as hints to a model checker wouldn't significantly add time to the development process. There is a lot of misinformation about the supposed overhead of formal methods or the assumption that they must be total. A lot of this is due to the fact that formal methods are typically used in projects where serious harm or death could result from an error, and thus, the cost of verifying the project must exceed the potential risk for failure. An incremental approach is pragmatic and yields results. It does not require the totality of classical formal methods projects.
I have not noticed a significant increase in time for my own development efforts while incorporating model checking into my own software development process. That's anecdotal, but up until the past 10 years, there haven't been many OSS model checkers to choose from. In turn, the cost for using these tools have made it harder to have peer reviewed cost analysis. If using a commercial tool, it's because you were required to do so via FAA or similar. Hence, the adoption curve hasn't yet been wide enough to have the wide adoption you've alluded to. With both the availability of these tools and the increased tool-assisted nature of red team analysis, it'll come.
Language can't fix these problems alone. Not even Rust is a silver bullet here. But, the incorporation of formal methods is an additional layer in a good defense-in-depth strategy.
Yes, it would.
void z(value* x) {
/* preconditions. */
MODEL_ASSERT(valid_value_ptr(x));
/* do operation. */
/* postconditions. */
MODEL_ASSERT(z_postcondition(a, b));
}
In practice: z(NULL); /* model failure. */
z(&uninitialized_value_on_stack); /* model failure. */
It works, and the model checker runs in seconds.