
LPE and RCE in OpenBSD OpenSMTPD - aquabeagle
https://www.openwall.com/lists/oss-security/2020/01/28/3
======
Panino
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.

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

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

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

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

[https://man.openbsd.org/syspatch](https://man.openbsd.org/syspatch)

[https://www.openbsd.org/errata66.html#p018_smtpd_tls](https://www.openbsd.org/errata66.html#p018_smtpd_tls)

[https://www.openbsd.org/errata66.html#p019_smtpd_exec](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...](https://www.mail-
archive.com/misc@opensmtpd.org/msg04850.html)

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

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

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

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

~~~
jacobush
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);
        }

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

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

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

~~~
angry_octet
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/](https://oxide.computer/blog/tags/podcast/)

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

~~~
pabs3
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-...](https://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-
society/)

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

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

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

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

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

~~~
codezero
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
:)

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

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

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

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

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

------
LeonM
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](https://www.openbsd.org/errata65.html)

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

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

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

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

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

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

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

