16 ms·

An unexpected Redis sandbox escape affecting Debian-based distros

154 points by reginaldo 5y ago

goodpoint 5y ago
Unbundling dependencies is a security feature.
reginaldo 5y ago
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.
- booi 5y ago
  oof, and just for people who are curious but this affects Ubuntu / Ubuntu server as well being a Debian derivative.
- Fabricio20 5y ago
  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 5y ago
    Most people probably don't use Redis authentication, and most servers are configured to not require it (including the Debian/Ubuntu defaults).
junon 5y ago
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 5y ago
  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 5y ago
    Ohhh okay I see. Wow :( Thanks for the clarification.
trasz 5y ago
So it’s about escaping the sandbox around Lua interpreter, not escaping some sandbox Redis itself is running in?
- gmfawcett 5y ago
  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 5y ago
  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.
qalmakka 5y ago
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.
- b112 5y ago
  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 5y ago
    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.
    - gmfawcett 5y ago
      That's a notable prior example from 14 years ago. I'm not sure you're making a strong argument here!
      - pgporada 5y ago
        It's still relevant for Web PKI work.
        yjftsjthsd-h 5y ago
        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?
    - gunapologist99 5y ago
      Another similar one (perhaps worse!) from the same era: https://jblevins.org/log/ssh-vulnkey https://jblevins.org/log/ssh-vulnkey
josephcsible 5y ago
Why did Debian need to change Redis to link Lua dynamically in the first place?
- gspr 5y ago
  Unnecessary static linking is a Policy violation.
- blihp 5y ago
  Dynamic linking is Debian's preferred approach. When it works as intended (which is usually), it's generally a good thing.
- infogulch 5y ago
  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 5y ago
    > 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 5y ago
    - forty 5y ago
      You missed the second part of the message that makes it clear it's sarcasm :)
- rlpb 5y ago
  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.
unixbane 5y ago
omg im gonna buy akamai products now
mherrmann 5y ago
Debian's unattended-upgrades is amazing. Just checked some of my boxes and they are already running the patched version.
- forty 5y ago
  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.
benmmurphy 5y ago
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/49efe300af258e83f377cd8142d2c67d66fc2e3a 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/ldo.c 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.
- bawolff 5y ago
  Given that lua is meant to be an embedded language, its kind of surprising this is neccesary.
  - tedunangst 5y ago
    Making an application scriptable doesn't always mean you want it to be scriptable by hostile adversaries.
    - bawolff 5y ago
      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.
- mikemike 5y ago
  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 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.
gunapologist99 5y ago
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 5y ago
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 https://redis.io/topics/programmability
- tedunangst 5y ago
  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 5y ago
    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.
- mikemike 5y ago
  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").

154 points by reginaldo 5y ago

Unbundling dependencies is a security feature.

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.

booi 5y ago
oof, and just for people who are curious but this affects Ubuntu / Ubuntu server as well being a Debian derivative.
Fabricio20 5y ago
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 5y ago
  Most people probably don't use Redis authentication, and most servers are configured to not require it (including the Debian/Ubuntu defaults).

junon 5y ago

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 5y ago
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 5y ago
  Ohhh okay I see. Wow :( Thanks for the clarification.

trasz 5y ago

So it’s about escaping the sandbox around Lua interpreter, not escaping some sandbox Redis itself is running in?

gmfawcett 5y ago
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 5y ago
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.

qalmakka 5y ago

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.

b112 5y ago
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 5y ago
  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.
  - gmfawcett 5y ago
    That's a notable prior example from 14 years ago. I'm not sure you're making a strong argument here!
    - pgporada 5y ago
      It's still relevant for Web PKI work.
      - yjftsjthsd-h 5y ago
        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?
  - gunapologist99 5y ago
    Another similar one (perhaps worse!) from the same era: https://jblevins.org/log/ssh-vulnkey https://jblevins.org/log/ssh-vulnkey

josephcsible 5y ago

Why did Debian need to change Redis to link Lua dynamically in the first place?

gspr 5y ago
Unnecessary static linking is a Policy violation.
blihp 5y ago
Dynamic linking is Debian's preferred approach. When it works as intended (which is usually), it's generally a good thing.
infogulch 5y ago
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 5y ago
  > 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 5y ago
  - forty 5y ago
    You missed the second part of the message that makes it clear it's sarcasm :)
rlpb 5y ago
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.

unixbane 5y ago

omg im gonna buy akamai products now

mherrmann 5y ago

Debian's unattended-upgrades is amazing. Just checked some of my boxes and they are already running the patched version.

forty 5y ago
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.

benmmurphy 5y ago

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/49efe300af258e83f377cd8142d2c67d66fc2e3a 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/ldo.c 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.

bawolff 5y ago
Given that lua is meant to be an embedded language, its kind of surprising this is neccesary.
- tedunangst 5y ago
  Making an application scriptable doesn't always mean you want it to be scriptable by hostile adversaries.
  - bawolff 5y ago
    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.
mikemike 5y ago
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 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.

gunapologist99 5y ago

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 5y ago

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 https://redis.io/topics/programmability

tedunangst 5y ago
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 5y ago
  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.
mikemike 5y ago
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").