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.
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.
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.
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.
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.
That's what i mean. For an embedded scriptable language, making something scriptable and sandboxed is a core use case. It is surprising to me that doing the thing lua is famous for requires source code mods.
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.)
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.
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.
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..
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.
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.
Debian really hates code duplication and prefers risking security issues resulting from trying to reduce code duplication. This is not the first time this sort of thing has happened, and has caused security issues.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
Unfortunately, Debian will sometimes also patch the package to remove failing tests, not understanding that it is a security issue. This is not the first time this has happened.
This is also sometimes unavoidable because not all test suites are built very robustly. One often comes across projects with test suites that the author only intended for their own use and doesn't care if it fails on other peoples systems.
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.
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.