8 ms·

Redis Lua scripting: several security vulnerabilities fixed

59 points by itamarhaber 8y ago

mathnmusic 8y ago
> Honestly when the Redis Lua engine was designed, it was not conceived with this security model of the customer VS the cloud provider in mind. The assumption kinda was that you can trust who pokes with your Redis server. So in general the Lua libraries were not scrutinized for security. The feeling back then was, if you have access to Redis API, anyway you can do far worse. This is an interesting point. Cloud computing and managed/hosted services require a clear separation of what the host can do and what the customer (who's paying for the managed service) should be able to do. Just today, our startup decided to use AWS Kinesis (as opposed to setting up Kafka ourselves), despite the vendor lock-in and closed-source nature of AWS components. :-/
- zokier 8y ago
  Yeah, these sorts of questions are typically something that goes under the umbrella of "multi-tenancy"; having proper access controls, isolation, accounting, logging, security model, etc, which incidentally also makes even trivial software explode with complexity. I haven't been paying attention, but I would have not thought Redis to be multi-tenant ready out of the box.
samatman 8y ago
Consensus in the community is that Lua(JIT) sandboxing must be done on the process level. Even with the debug library stripped and other safeguards against (inner) evaluation taken, the trivial DOS of `while true do end` remains. If that happens, you want it to live in its own process, or at least its own thread.
- antirez 8y ago
  Those kind of dos (while true do end), actually we handle quite well already, because we don't use LuaJIT but plain Lua that has hooks that allow Redis to detect such conditions. But in general, it's a "can of worms" indeed.
  - andrewmcwatters 8y ago
    iirc and ime, that sort of provision in Lua only allows you keep running at a cost. You'll still encounter hitching because infinite loop detection is based on a detection of number of line executions.
    - antirez 8y ago
      Yes, but in the case of Redis the fact that an user makes its own Redis server slower is not an issue basically. It is enough that we can detect this and kill the script.
  - cjcole 8y ago
    Are your Lua changes public? I couldn't find a relevant repo. Thanks.
    - aarongolliver 8y ago
      If you have the time he has a great video here[0] that talks about redis+LUA [0] https://www.youtube.com/watch?v=RaIwBagADVM https://www.youtube.com/watch?v=RaIwBagADVM
    - antirez 8y ago
      Yes but there is no direct modification to the Lua code, we just install the hooks from the Redis source code using the Lua interpreter. So the implementation of that is directly inside the Redis source tree, scripting.c file.
moby 8y ago
Heroku's updated their Redis fleet: https://blog.heroku.com/redis-vulnerability https://blog.heroku.com/redis-vulnerability
andrewmcwatters 8y ago
> To be fair, I think that the assumptions Lua makes about the stack are a bit too trivial, with the Lua library developer having to constantly check if there is enough space on the stack to push a new value. What the fuck? This is almost never a concern for Lua C developers. If you're concerned with LUA_MAXCSTACK defaulted at 2048 and you're running out of space, you're doing something seriously wrong and need to reevaluate how you're using the Lua C API.
- antirez 8y ago
  Hello Andrew, unfortunately the max limit is hit only if you make sure to call the appropriate function that will reallocate the stack if needed. I think that it would be saner for Lua, by default, just to accept stack pushes and resize the stack if needed. I've the feeling that this would not be a huge performance issue because most of the times it's just a check and no reallocation is needed. As I wrote in the blog post, note that other languages do not force you to think at such low level when using their C API. In general Lua implementation is great, but the API exposing the stack, really poses just a barrier that I feel is kinda pointless compared to providing the classical argv/argc API of Python, Ruby, Tcl, ...
  - ufo 8y ago
    The stack-based API is the trick that allows Lua to have much saner memory management than the alternatives. Unlike Python, C code using the Lua API does not need to manually keep track of reference counts. Unlike Ruby, Lua's garbage collector is accurate and can be coded in pure and portable C. I agree with you that needing to keep track of the stack size is a pain though. I also wish it would just grow on demand as needed. That said, when you are doing things "by hand" the default stack limit of 10 slots per function call frame is often enough. It could be worse :)
  - andrewmcwatters 8y ago
    No, it wouldn't be saner. You're completely ignoring memory allocation concerns. You should almost never reach 2048 values on the stack to begin with. It's intentionally arbitrarily high. Reviewing the commit in question: you have a function that blindly accepts any number of arguments. ~2047 arguments in a function is not an acceptable design choice for Lua. I can't see any language where I'd design an API for users where they could insert an upwards of ~2047 arguments and say to myself that's a good idea. Your misuse of the Lua C API here does not grant a special use case, nor does it justify entire language implementation changes. Your number one concern for hardening starts at the stack? And further more you think it's a good idea to compile a custom version of Lua it to avoid stack issues instead of just writing sane bindings? You would really go out of your way to do this? Maybe instead of saying there's a problem with X, Y or Z, ask yourself if you're doing something wrong first. Allow developers to pass in a table of values to pack instead of an arbitrary amount of arguments, push the values by index onto the stack, work with them, then pop them off. How is this difficult? What makes your use case special? Why would you avoid conventional Lua use? vararg (..., select, {...} convention) usage in Lua almost always involves the use of tables.
breakingcups 8y ago
I appreciate the (what feels like) honest and direct communication from Antirez very much. It's always such a breath of fresh air.
ksec 8y ago
Somewhat off topic: What Happened to the mRuby Scripting in Redis? I remember there were plans to make mRuby in Redis too. Given mRuby has had quite a bit of security audit in recent years that cost Shopify millions.

59 points by itamarhaber 8y ago

> Honestly when the Redis Lua engine was designed, it was not conceived with this security model of the customer VS the cloud provider in mind. The assumption kinda was that you can trust who pokes with your Redis server. So in general the Lua libraries were not scrutinized for security. The feeling back then was, if you have access to Redis API, anyway you can do far worse. This is an interesting point. Cloud computing and managed/hosted services require a clear separation of what the host can do and what the customer (who's paying for the managed service) should be able to do. Just today, our startup decided to use AWS Kinesis (as opposed to setting up Kafka ourselves), despite the vendor lock-in and closed-source nature of AWS components. :-/

zokier 8y ago
Yeah, these sorts of questions are typically something that goes under the umbrella of "multi-tenancy"; having proper access controls, isolation, accounting, logging, security model, etc, which incidentally also makes even trivial software explode with complexity. I haven't been paying attention, but I would have not thought Redis to be multi-tenant ready out of the box.

samatman 8y ago

Consensus in the community is that Lua(JIT) sandboxing must be done on the process level. Even with the debug library stripped and other safeguards against (inner) evaluation taken, the trivial DOS of `while true do end` remains. If that happens, you want it to live in its own process, or at least its own thread.

antirez 8y ago
Those kind of dos (while true do end), actually we handle quite well already, because we don't use LuaJIT but plain Lua that has hooks that allow Redis to detect such conditions. But in general, it's a "can of worms" indeed.
- andrewmcwatters 8y ago
  iirc and ime, that sort of provision in Lua only allows you keep running at a cost. You'll still encounter hitching because infinite loop detection is based on a detection of number of line executions.
  - antirez 8y ago
    Yes, but in the case of Redis the fact that an user makes its own Redis server slower is not an issue basically. It is enough that we can detect this and kill the script.
- cjcole 8y ago
  Are your Lua changes public? I couldn't find a relevant repo. Thanks.
  - aarongolliver 8y ago
    If you have the time he has a great video here[0] that talks about redis+LUA [0] https://www.youtube.com/watch?v=RaIwBagADVM https://www.youtube.com/watch?v=RaIwBagADVM
  - antirez 8y ago
    Yes but there is no direct modification to the Lua code, we just install the hooks from the Redis source code using the Lua interpreter. So the implementation of that is directly inside the Redis source tree, scripting.c file.

moby 8y ago

Heroku's updated their Redis fleet: https://blog.heroku.com/redis-vulnerability https://blog.heroku.com/redis-vulnerability

andrewmcwatters 8y ago

> To be fair, I think that the assumptions Lua makes about the stack are a bit too trivial, with the Lua library developer having to constantly check if there is enough space on the stack to push a new value. What the fuck? This is almost never a concern for Lua C developers. If you're concerned with LUA_MAXCSTACK defaulted at 2048 and you're running out of space, you're doing something seriously wrong and need to reevaluate how you're using the Lua C API.

antirez 8y ago
Hello Andrew, unfortunately the max limit is hit only if you make sure to call the appropriate function that will reallocate the stack if needed. I think that it would be saner for Lua, by default, just to accept stack pushes and resize the stack if needed. I've the feeling that this would not be a huge performance issue because most of the times it's just a check and no reallocation is needed. As I wrote in the blog post, note that other languages do not force you to think at such low level when using their C API. In general Lua implementation is great, but the API exposing the stack, really poses just a barrier that I feel is kinda pointless compared to providing the classical argv/argc API of Python, Ruby, Tcl, ...
- ufo 8y ago
  The stack-based API is the trick that allows Lua to have much saner memory management than the alternatives. Unlike Python, C code using the Lua API does not need to manually keep track of reference counts. Unlike Ruby, Lua's garbage collector is accurate and can be coded in pure and portable C. I agree with you that needing to keep track of the stack size is a pain though. I also wish it would just grow on demand as needed. That said, when you are doing things "by hand" the default stack limit of 10 slots per function call frame is often enough. It could be worse :)
- andrewmcwatters 8y ago
  No, it wouldn't be saner. You're completely ignoring memory allocation concerns. You should almost never reach 2048 values on the stack to begin with. It's intentionally arbitrarily high. Reviewing the commit in question: you have a function that blindly accepts any number of arguments. ~2047 arguments in a function is not an acceptable design choice for Lua. I can't see any language where I'd design an API for users where they could insert an upwards of ~2047 arguments and say to myself that's a good idea. Your misuse of the Lua C API here does not grant a special use case, nor does it justify entire language implementation changes. Your number one concern for hardening starts at the stack? And further more you think it's a good idea to compile a custom version of Lua it to avoid stack issues instead of just writing sane bindings? You would really go out of your way to do this? Maybe instead of saying there's a problem with X, Y or Z, ask yourself if you're doing something wrong first. Allow developers to pass in a table of values to pack instead of an arbitrary amount of arguments, push the values by index onto the stack, work with them, then pop them off. How is this difficult? What makes your use case special? Why would you avoid conventional Lua use? vararg (..., select, {...} convention) usage in Lua almost always involves the use of tables.

breakingcups 8y ago

I appreciate the (what feels like) honest and direct communication from Antirez very much. It's always such a breath of fresh air.

ksec 8y ago

Somewhat off topic: What Happened to the mRuby Scripting in Redis? I remember there were plans to make mRuby in Redis too. Given mRuby has had quite a bit of security audit in recent years that cost Shopify millions.