Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sandbox subverting via global environment manipulation (ineffective whitelisting) #2854

Open
lucab opened this issue Nov 5, 2015 · 2 comments
Labels

Comments

@lucab
Copy link

@lucab lucab commented Nov 5, 2015

[re-posting via github after private reporting, as agreed with antirez]

It is general LUA wisdom that sandboxing would be better implemented by explicitly whitelisting just things that should be exposes, instead of blacklisting some functions/tables as redis is currently doing.

From a quick glance, there are several functions exposed by redis (in both 2.8 and 3.0 branches) which looks dangerous. For example, all of the following ones look un-uneeded in redis:

  • rawget, rawset, rawequal
  • setfenv, getfenv
  • getmetatable, setmetatable
  • loadstring, load
  • newproxy

There are probably some more, and some can get added/removed as LUA evolves. The key point is that lua internals should probably be all hidden by default, and only needed functions picked and re-exported.
In fact, This leaky whitelist approach make it easier to subvert the sandbox in different ways.

For example, the whole "strict lua" in scriptingEnableGlobalsProtection() can be bypassed with a simple setmetatable(_G, nil).

Another example is internal de-synchronization reported in #2853, resulting in remote crash due to assertion hitting.

@antirez antirez added the scripting label Nov 6, 2015
@lucab

This comment has been minimized.

Copy link
Author

@lucab lucab commented Nov 6, 2015

Digging through the archive, I just realized that is report is closely related (but not completely identical) to #1725.

@lucab

This comment has been minimized.

Copy link
Author

@lucab lucab commented Nov 6, 2015

After further review, MITRE agreed in not categorizing this as a vulnerability report, as "[redis lua] sandboxing is not (yet) intended to define a security boundary with any practical value".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.