Hacker News new | comments | show | ask | jobs | submit login
Redis Lua scripting: several security vulnerabilities fixed (antirez.com)
59 points by itamarhaber 62 days ago | hide | past | web | favorite | 22 comments



> 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. :-/


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.


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.


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.


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.


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.


Are your Lua changes public? I couldn't find a relevant repo. Thanks.


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.


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


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


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


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.


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


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


The reasoning behind not automatically growing the stack was given by the Lua authors recently: http://lua-users.org/lists/lua-l/2018-04/msg00036.html


Thanks for the relevant info.


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


Lua's minimalist design philosophy means everything is a tradeoff. If there's a way to write an allocator which can handle a growing stack, while adding only ~10k to the runtime, I think you could get buy-in on that.

Keeping the stack on the lua_State has clear advantages, such as allowing the library to be reentrant. Handling malloc and free in a single pass for each lua_State is simpler than alternatives and fits well-enough with existing practices.


I am curious what kind of workload is prompting you to want this. I've only ever seen stack overflows from predictable recursive errors, which is generally a pretty shallow bug.

Second observation: LuaJIT might be the better platform to pursue this approach. Since the tracer is already on, stack overflows could potentially be another cold path that gets handled on spill; performance impact could be minimal, perhaps none if existing trace guards can be used.


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.


Your aggressive, insulting communication style is making you less effective and undermining your own goals with communicating.


I'm fully aware of the tone I'm passing off: take it or leave it. I have no vested interest in this. I'm simply a gawking spectator. Learn to accept input from others who don't give it to you on a silver platter.

The otherwise timid and passive voice on HN is frequently an irritation to me. This is the author of Redis making an uninformed decision in both writing Lua bindings, and extending that ignorance to making changes to Lua to suit his ignorance. I don't care. What my tone is suggesting is how ludicrous this is and how it shouldn't be taken as a good suggestion.

This is a great example of someone who has a large exposure simply using something incorrectly and him having a large exposure doesn't make his technical observations of a language meaningful. In no way here can you leverage that to justify poor decision making.




Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact

Search: