LPE and RCE in OpenBSD OpenSMTPD | Hacker News Reader

LPE and RCE in OpenBSD OpenSMTPD

openwall.com

94 pointsaquabeagle83 comments

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.

This RCE is trivial and super bad.

That's for pointing this out.

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.

No data, but it only listens on localhost by default.

Do you think anyone who changes the default also changes the delivery to Maildir?

(So, at a minimum, it's a local user privilege escalation, of any user to root, in the default install?)

>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.

> for free

It's only free from external cost though. The time for the staff at Qualys is (probably) not free. ;)

Also just in - they eat and breathe. :) And the heat-death of the Universe is closer this Monday.

> Is Qualys getting paid for this excellent > work, and if so, by who?

See:

https://en.wikipedia.org/wiki/Qualys

My guess is they use OpenBSD and this is part of 'give back'.

OpenBSD errata and binary syspatches are already available for 6.5/6.6.

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...

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.

I will add, this bug occurred essentially because of the following logic bug;

  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.

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.

I feel like the "canonical" example of using pattern matching for nested if statements is fizzbuzz

    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.

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!

Couldn't you do this with an anonymous function in languages which have those? Which would look like:

    {
        if A && B return println("FizzBuss");
        if A && !B return println("Fizz");
        if !A && B return println("Buzz");
        if !A && !B return println("{}", i);
    }

that just seems to being us back to the problem of not wanting to re-evaluate the functions...

Pattern matching has been added in C# 8.0. See: https://docs.microsoft.com/en-us/dotnet/csharp/pattern-match...

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:

    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.

C supports the following, which is short, correct, and easy to understand:

  save_A = A;
  save_B = B;
  if (!A)
    fix(A);
  if (!B)
    fix(B);
  return (some_expression of save_A and save_B);

That's an odd example, but you can do it in OCaml:

match (a, b) with | (1, 1) -> 1 | (1, 0) -> (fix b); 1 | _ -> 0

In C:

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;

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.

Haskell would due to lazy evaluation.

Is it guaranteed to be lazy (in this specifically useful way)? Or is it just not guaranteed to be eager?

I usually try to flatten the branches, using `else if` when a `switch` statement isn't viable.

  if (A && B) {
    return 1
  else if (A && !B) {
    fix(B)
    return 1
  else {
    return 0
  }

You could do that in C by shifting the bools to make a bitfield.

Also, you may want to add that when it's patched, the response to the MAIL FROM command is:

    553 5.1.0 Sender address syntax error

I tried this on my smtpd but I couldn't make it create a file. At what point would the command get executed?

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?)

https://oxide.computer/blog/tags/podcast/

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?

My question is why are they using /bin/sh at all? system() and friends that execute /bin/sh are almost always a source of vulnerabilities.

https://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-...

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.

Restricted mode provides hardly any protection against untrusted code.

Also, it's a bashism. It's not implemented by the OpenBSD's /bin/sh.

openBSD uses mksh which includes a restricted mode (http://www.mirbsd.org/htman/i386/man1/mksh.htm)

Oh? https://man.openbsd.org/sh.1 doesn't mention -r.

OpenBSD started out as pdksh I believe, not mksh. But it still has a restricted mode.

https://man.openbsd.org/ksh.1#AUTHORS

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.

The root of the issue is relying on ad-hoc string escaping for security.

I love the copious references to the 80s film WarGames in the examples :)

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?

Theo says Web facing things applications is a good place to start using rust, he proved himself right.

Dude this rhymes... Could this be the lyrics for next Openbsd release song?

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.

[0] https://www.openbsd.org/errata65.html

The oss-security@ mailing list is hosted by openwall. This advisory was posted by Qualys Security.

Darn, you are right! I need sleep... But first let me patch my boxes.

Thanks for the tip about needing the official CDN!

It's /etc/installurl not /etc/updateurl.

I really wish the OpenBSD team would consider rewriting some of these internet facing services in Go. It would be much safer than C.

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).

It's not idiomatic C, and is pretty rare in practice. And the ergonomics are vastly better in other languages, starting with C++.

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.

But you still have to decide what is invalid in order to do that. You could just as easily not think of an invalid input case.

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.

Many things, including Rust's type system, could theoretically prevent bugs like this if used properly too. The question comes down to compatibility of these tools with normal development practices. Formal methods are not known for their wide use in software like OpenSMTPD for many reasons, despite being around for decades. Until that changes, I have a hard time believing they are a "pragmatic" solution.

People say this all the time, but in practice, no, Rust's type system will not be used to prevent logic bugs, any more than model-checked C will be used to prevent random bugs in OpenBSD's C code. This simply isn't a bug that's in Rust's remit to address. It's a logic bug. Your language has those as much as anyone else's.

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.

> People say this all the time, but in practice, no, Rust's type system will not be used to prevent logic bugs, any more than model-checked C will be used to prevent random bugs in OpenBSD's C code.

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.)

I would call it more of a professional horse.

OpenBSD's development process already incorporates linters and other static analysis tools, as well as code reviews.

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.

> Decomposing logic using Hoare triplets and properly incorporating assertions as hints to a model checker wouldn't significantly add time to the development process.

Yes, it would.

I do it today; your assumption that "it would" is incorrect.

    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.

>The class of exploits that are currently being discovered are often discovered via tool-assisted analysis

Which tools would I start googling if I wanted to learn more about this?

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.

I'm not arguing against their use.

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.

If it's in base, that would mean dragging Go itself into the OpenBSD tree. Which you can argue is good, but that's a major change.

nice, then someone would just port rust and go and whatever to all the obsd arches, and not just to the main 2-3 platforms. Breaths not being held.

At least it's BSD-licensed.

Reply on news.ycombinator.com