[HN Gopher] An unexpected Redis sandbox escape affecting Debian-...
       ___________________________________________________________________
        
       An unexpected Redis sandbox escape affecting Debian-based distros
        
       Author : reginaldo
       Score  : 122 points
       Date   : 2022-03-09 14:32 UTC (8 hours ago)
        
 (HTM) web link (www.ubercomp.com)
 (TXT) w3m dump (www.ubercomp.com)
        
       | benmmurphy wrote:
       | I think this is still 'broken' because Redis have applied custom
       | patches to the Lua source code in order to prevent sandbox
       | escapes. If Debian Redis is using the normal Lua then these
       | patches will not have been applied.
       | 
       | https://github.com/redis/redis/commit/49efe300af258e83f377cd...
       | 
       | and you can check the history on that file to see they haven't
       | removed the fix:
       | 
       | https://github.com/redis/redis/commits/unstable/deps/lua/src...
       | 
       | it could be the latest lua version that Debian ships with has
       | 'safe' byte code evaluation and this fix no longer needs to be
       | applied.
        
         | mikemike wrote:
         | Yes, of course it's vulnerable, verified with Docker
         | debian:sid. That was my first reaction when I read this, but I
         | wanted to verify it first. You beat me with this post.
         | 
         | Since you've already let the cat out of the hat (which is not
         | ideal), please file the bugs at Debian and Ubuntu.
         | 
         | Test command:                   redis-cli eval 'return
         | select(2, loadstring("\027")):match("binary") and "VULNERABLE"
         | or "OK"' 0
         | 
         | While we're at it, redis has ignored the advice at: http://lua-
         | users.org/wiki/SandBoxes Almost all of the critical functions
         | (loadstring, load, getmetatable, getfenv, ...) are present and
         | unprotected in the redis 'SandBox' (which isn't).
         | 
         | Which means, disable scripting or shut down your redis
         | instances NOW, which do not run with the same privileges as any
         | client which has access to this. Scripting can be disabled by
         | renaming the EVAL and EVALSHA commands to unguessable names.
        
           | josephcsible wrote:
           | While you're right about the binary loadstring issue, that
           | lua-users page is way overly paranoid. The best Lua
           | sandboxing implementation I know of is the one Wikipedia
           | uses, and it allows a lot of what's "unsafe" there.
        
             | adamc wrote:
             | Not really usually to call something "overly paranoid"
             | without going into why you think their evaluation is wrong.
        
               | tedunangst wrote:
               | The page is trying to build a sandbox where a lua script
               | can eval other untrusted lua code within the same lua
               | execution environment. Many, even most?, people are only
               | interested in isolating the host application from the lua
               | environment.
        
           | reginaldo wrote:
           | I reported the "loadstring accepts arbitrary bytecode" issue
           | back to Debian and Ubuntu.
           | 
           | But, just to clarify, mikemike's advice is that everyone
           | (i.e. including people running upstream redis) should disable
           | scripting.
        
           | tedunangst wrote:
           | I don't think getmetatable et al are really problems, are
           | they? You can mess things up in the sandbox, but that's not
           | escaping it. I think that page is trying to build a sandbox
           | where even a lua script can eval other lua code without
           | blowback, but that's not what redis is trying to achieve.
        
         | bawolff wrote:
         | Given that lua is meant to be an embedded language, its kind of
         | surprising this is neccesary.
        
           | tedunangst wrote:
           | Making an application scriptable doesn't always mean you want
           | it to be scriptable by hostile adversaries.
        
       | qalmakka wrote:
       | Yet another issue due to nonsensical Debian packaging policies.
       | Debian should just stop wasting time applying random patches or
       | modifying packages, if upstream is doing something there's
       | probably a good reason for that.
        
         | [deleted]
        
         | reginaldo wrote:
         | I didn't go there at any point while writing the article,
         | because, for a Linux distro (i.e. something that tries to be
         | all things to all people), I think Debian does a great job, and
         | the packaging policy is a net positive, by a wide margin. These
         | things happen sometimes, and Chris Lamb (the package maintainer
         | and Debian / reproducible builds extraordinaire), as well as
         | both the Debian and Ubuntu teams, handled them very well.
         | 
         | When running large scale production services, the best practice
         | is building your own image, perhaps starting minimal Debian or
         | Alpine core, using automation, and the tech giants have settled
         | on statically linked binaries a while ago, so Debian packaging
         | "idiosyncrasies" should not apply anyways.
        
         | ptx wrote:
         | Maybe upstream shouldn't make their security measures so
         | fragile that everyone downstream has to resign themselves to
         | not touching anything. Open source software is supposed to be
         | modified and adapted.
         | 
         | Is this Lua sandboxing really a good idea? It it robust? It
         | didn't work out well for Java, which removed the
         | SecurityManager feature recently. Python gave up on attempts at
         | sandboxing many decades ago.
        
         | ris wrote:
         | The real bug here is not having a test for this, or not running
         | the tests as part of the packaging process. We should not have
         | situations where packages are so sensitive that a small change
         | to them should be considered unsafe. Open Source is pointless
         | if your source is regarded as immutable. If there _is_ a subtle
         | detail like this (and sometimes it 's unavoidable), it should
         | be covered by a test so that ensuring the end result is "ok" is
         | nothing more than seeing the tests pass.
         | 
         | I'm not going to weigh in on which party should be responsible
         | for that test.
        
         | bbarnett wrote:
         | Because all of the issues they've genuinely fixed, security and
         | otherwise, are meaningless.
         | 
         | Or are you just basing this on <bad thing> happened, thus all
         | other cases (hundred of thousands of patches) are wrong?
        
           | josephcsible wrote:
           | This isn't the only time Debian has introduced a serious
           | security vulnerability by changing things in packages. The
           | most notable prior example that comes to mind is
           | CVE-2008-0166.
        
             | gunapologist99 wrote:
             | Another similar one (perhaps worse!) from the same era:
             | https://jblevins.org/log/ssh-vulnkey
        
               | josephcsible wrote:
               | Isn't that the same one?
        
             | gmfawcett wrote:
             | That's a notable prior example from _14 years ago_. I 'm
             | not sure you're making a strong argument here!
        
               | pgporada wrote:
               | It's still relevant for Web PKI work.
        
               | yjftsjthsd-h wrote:
               | How is it still relevant? Even if certs made with a
               | vulnerable version weren't revoked at the time, wouldn't
               | they would have been rotated by now?
        
         | gspr wrote:
         | If you think the patches are _random_ , you must be missing
         | something.
        
         | rlpb wrote:
         | Distributions continuously patch packages. It's necessary to be
         | able to ship a working, integrated distribution at all. It's
         | also necessary because users expect some level of consistency
         | and integration between components, and distributions are the
         | spaces where that can be achieved. Usually these achievements
         | then get upstreamed so it's easy to forget about their origin.
         | There's also the cases where distribution users just
         | fundamentally disagree with upstream policy and use their
         | ability to change Free Software to make it happen - such as the
         | disabling of auto-updates in distribution packages coming
         | outside the distribution packaging system, or the disabling of
         | [opt-out] telemetry.
         | 
         | So there are many good reasons distributions patch. It's not
         | reasonable to paint them all with the same brush. If you want
         | to credibly criticize this, you need to be more specific.
        
           | josephcsible wrote:
           | Isn't a major selling point of Arch that it works fine
           | without patching packages?
        
             | yjftsjthsd-h wrote:
             | I think it would be more accurate to say that Arch tends to
             | do less patching than other distros, sometimes even none
             | but not always.
        
             | chadcatlett wrote:
             | Arch patches Redis, although not for lua.
             | https://github.com/archlinux/svntogit-
             | community/blob/package...
        
       | junon wrote:
       | Mrh why does Redis include the package library? IMO packages
       | should be loaded at boot via configuration. This is Lua hardening
       | 101...
       | 
       | Good writeup. Thank you.
        
         | reginaldo wrote:
         | Upstream Redis doesn't. At least in my builds from upstream,
         | luaopen_package and luaopen_os aren't even available on the
         | binary. Debian does things differently, though. They need
         | package in order to require the "json" and "bit" libraries,
         | which are packaged separately on Debian.
         | 
         | Another thing that would be interesting and affect upstream as
         | well would be getting the "debug" package back from the redis
         | error function.
        
           | junon wrote:
           | Ohhh okay I see. Wow :( Thanks for the clarification.
        
       | trasz wrote:
       | So it's about escaping the sandbox around Lua interpreter, not
       | escaping some sandbox Redis itself is running in?
        
         | gmfawcett wrote:
         | Right -- you can escape the sandbox that Lua runs in, inside
         | Redis, and get the Redis process' user to execute arbitrary
         | code in its environment. Redis might be inside a hardened
         | container, or running as a privileged account on a shared
         | machine.
        
         | luto wrote:
         | redis-server is just a plain executable. Unless you're putting
         | it in a sandbox yourself - for example, using a VM or systemd
         | features - redis has no sandbox. There is nothing to escape
         | from.
         | 
         | The Lua interpreter _within_ redis acts as a sandbox, which got
         | a bit mangled here. Most features are not implemented in Lua,
         | though. So this is only used for things like the EVAL command.
        
       | goodpoint wrote:
       | Unbundling dependencies is a security feature.
        
       | reginaldo wrote:
       | Author here, so feel free to ask me anything. I had to shrink the
       | title in order for it to fit, but, as the first paragraph says,
       | this affects only Debian and Debian-based distros, so it's a
       | Debian bug, not a Redis bug.
        
         | Fabricio20 wrote:
         | If I understand correctly this RCE is mostly a big concern to
         | hosts that don't make use of redis authentication, as eval is
         | locked behind it? Some quick testing on my end suggests the
         | eval command is parsed before authentication check but requires
         | authentication for execution, but I may be wrong here.
         | 
         | In a perfect world we can all update immediately but when we
         | still have some servers in Ubuntu 16.04..
        
           | gunapologist99 wrote:
           | Most people probably don't use Redis authentication, and most
           | servers are configured to not require it (including the
           | Debian/Ubuntu defaults).
        
         | booi wrote:
         | oof, and just for people who are curious but this affects
         | Ubuntu / Ubuntu server as well being a Debian derivative.
        
       | gunapologist99 wrote:
       | Does Redis upstream still bind to all addresses by default on
       | startup, or only to localhost?
       | 
       | One nice thing is that some years ago, Debian/Ubuntu configured
       | the default config to only bind to localhost. (You can change it,
       | but only if you need to, so it's "default secure" instead of
       | "default insecure".)
       | 
       | That can really hurt, especially since you might build it and
       | start it up for testing from within your personal user account
       | (maybe just to benchmark a new server -- and perhaps you have
       | sudo?), and then: boom, that entire machine is now pwned.
       | 
       | Binding to all interfaces by default is definitely not the wisest
       | design decision when you could just bind to localhost and then
       | let people bind to 0.0.0.0 as they like, but you still see some
       | naive devs doing it even on new software (like seaweedfs.) Even
       | Mongodb doesn't do that anymore.
        
       | ptx wrote:
       | Is this sandbox intended as a security feature?
       | 
       | According to the Redis documentation [1], "The sandbox attempts
       | to prevent accidental misuse [...] Scripts should never [...]
       | attempt to perform any other system call other than those
       | supported by the API".
       | 
       | (Also "reduce potential threats", but that doesn't sound like the
       | primary purpose or a particularly strong claim of security.)
       | 
       | [1] https://redis.io/topics/programmability
        
         | mikemike wrote:
         | That's what I'm wondering, too, right now.
         | 
         | It's trivial to DoS-hang redis with the script feature (and
         | SCRIPT KILL won't help).
         | 
         | And I found at least 3 DoS-crash, because it hasn't backported
         | fixes to its copy of Lua 5.1.5 (but Debian's liblua 5.1 might
         | -- I haven't checked).
         | 
         | And that's without even exploring the really problematic
         | builtins it still has available.
         | 
         | Maybe they should instead clarify their security guarantee for
         | redis scripting (e.g. "none").
        
         | tedunangst wrote:
         | I think your edits are mangling the intended meaning of the
         | quote. Your script should not try to perform other system
         | calls, because it won't work.
        
           | ptx wrote:
           | But does "it won't work" mean "don't try this in your app" or
           | does it mean that the system can be expected to safely run
           | arbitrary untrusted code?
           | 
           | The documentation makes it sound like it's primarily intended
           | to guide people towards using the API correctly. Here is the
           | whole section for context:
           | 
           | > _Redis places the engine that executes user scripts inside
           | a sandbox. The sandbox attempts to prevent accidental misuse
           | and reduce potential threats from the server 's environment._
           | 
           | > _Scripts should never try to access the Redis server 's
           | underlying host systems, such as the file system, network, or
           | attempt to perform any other system call other than those
           | supported by the API._
           | 
           | > _Scripts should operate solely on data stored in Redis and
           | data provided as arguments to their execution._
        
       | unixbane wrote:
       | omg im gonna buy akamai products now
        
       | josephcsible wrote:
       | Why did Debian need to change Redis to link Lua dynamically in
       | the first place?
        
         | gspr wrote:
         | Unnecessary static linking is a Policy violation.
        
         | blihp wrote:
         | Dynamic linking is Debian's preferred approach. When it works
         | as intended (which is usually), it's generally a good thing.
        
         | infogulch wrote:
         | Don't you know? Dynamic linking is _always_ better for
         | security. This is a great example. With dynamic linking you can
         | more quickly patch security holes caused by dynamic linking.
        
           | josephg wrote:
           | > Dynamic linking is always better for security
           | 
           | Always seems a bit strong given Debian's policy of dynamic
           | linking is responsible for this security vulnerability.
        
             | arpa wrote:
        
             | forty wrote:
             | You missed the second part of the message that makes it
             | clear it's sarcasm :)
        
         | rlpb wrote:
         | If everything were statically linked, getting your daily
         | security update would basically involve redownloading the
         | entire distribution, since when a core component were patched
         | everything would have to be rebuilt. This wouldn't be
         | practical. Therefore, dynamic linking is the norm on binary
         | distributions.
        
       | mherrmann wrote:
       | Debian's unattended-upgrades is amazing. Just checked some of my
       | boxes and they are already running the patched version.
        
         | forty wrote:
         | The only case I had problems with unattended upgrade was with
         | redis... Both the master and the replica restarted at the same
         | time, which caused some issues.
        
       ___________________________________________________________________
       (page generated 2022-03-09 23:01 UTC)