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

Allow installing some but not all of the standard modules. #55

Closed
milochristiansen opened this issue Nov 7, 2015 · 16 comments
Closed

Comments

@milochristiansen
Copy link

Currently you can have all, or no standard modules. This produces problems when, for example, you need everything but os and io to be available.

The fix for this is trivial, just export the openXYZ functions so that users can install modules piecemeal if they wish.

@cathalgarvey
Copy link
Contributor

I would like exactly the same functionality; there are cases where you want to allow people to provide code to execute without exposing potentially dangerous OS functions.

Aside, to explain why this is valuable:

I'm using Gopher-Lua to write a mailing list driver that loads incoming mail from IMAP, does some Lua to it, and pushes it out by SMTP (I actually can't believe nobody's done an IMAP/SMTP mailing list system before, I can't find any hint of it..). The bit in the middle where the Lua happens is an event-loop written as a local file by the user; fine, no special security flaws there except those the list admin adds themselves, and I want io/os loaded.

However, I would also like list moderators to be able to send arbitrary Lua to admin the list to be run in a separate execution environment, so they'd add new members by passing a script consisting of something like memberDatabase:addSubscriber("foo@bar.com"). This would be trivial to hack together but at present would either require cutting out the entire standard library (ouch), or exposing io and os to attacks by anyone who can spoof an email header! It's no big deal if some asshat spoofs headers and messes with your mailing list database (particularly if you revision it..), but it's quite a big deal if someone uploads your home directory to the web with wget.

When I hit this stage in my own project I'll happily clone, make some changes to expose the various lib functions, and issue a PR, unless someone beats me to it. I really hope you'll consider accepting!

@yuin
Copy link
Owner

yuin commented Jan 13, 2016

Lua itself should be kept as small as possible.
This issue can be resolved by simply assigning a nil value to global variables.

L = lua.NewState()
L.DoString("coroutine=nil;debug=nil;io=nil;math=nil;os=nil;string=nil;table=nil")

Issue #27 and #11 would be helpful for you.

@cathalgarvey
Copy link
Contributor

Ok, I see what you're suggesting. My concern would be that through some Lua magic, if an attacker can outrun the garbage collector they might find a reference in a sea of _Gs, or something. What's the threat surface of simply reassigning tables to nil, in GopherLua?

@yuin
Copy link
Owner

yuin commented Jan 14, 2016

If a certain global variable has been deleted by reassigning just after creating a new LState, any references of it can not be found.

@cathalgarvey
Copy link
Contributor

Hrm, Ok. Is that also true of a freshly created "sandbox" thread, or would one need an orphan LState to enjoy the same guarantee?

@yuin
Copy link
Owner

yuin commented Jan 14, 2016

L = lua.NewState()
L.DoString("coroutine=nil;debug=nil;io=nil;math=nil;os=nil;string=nil;table=nil")
th = L.NewThread()
// L and th share the same global variables( = th is sandboxed).

L2 =  lua.NewState()
L2.DoString("coroutine=nil;debug=nil;io=nil;math=nil;os=nil;string=nil;table=nil")

// L and L2 do not share the same global variables(= L2 is NOT sandboxed). You need L.Dostring() again.

@cathalgarvey
Copy link
Contributor

Well, I was more concerned about this use-case, forgive me if it's naive:

InsecureL = lua.NewState()
secureL = InsecureL.NewThread()
secureL.DoString("io=nil; debug=nil; os=nil")

Of course, doing the reverse might be more sensible (edit, no; this would be totally insecure, sorry):

secureL = lua.NewState()
secureL.DoString("io = nil; debug = nil; os = nil")
insecureL = secureL.NewThread()
insecureL.OpenLibs()

@cathalgarvey
Copy link
Contributor

..the point being, again, that you could create a rich environment with lots of additional stuff, as well as the "essential" core libraries, and use the same environment to execute code you trust as well as code you don't trust.

..which reminds me that the "secure first, insecure later" option is untenable, because malicious code could overwrite globals in secureL which are then inherited by insecureL, potentially making "trusted" code execute nasty stuff in a privileged environment.

@yuin
Copy link
Owner

yuin commented Jan 14, 2016

Hmm, if you implement luaopen_XX in Lua5.1 compliant functionality, I'll merge it. (Sorry I have no time to work this task, so I'm a just hobby programmer...)

As you can see in the lint.c, luaopen_XX functions in 5.1 should be cfunction. Currently, openXXX in GopherLua are not cfunction(not LGFunction).

Tasks:

  • Define constants corresponds to LUA_XXXNAME
  • Rewrite openXXX functions as an LGFunction
  • Modify the OpenLibs() to use new constants and new openXXX functions

@cathalgarvey
Copy link
Contributor

So, if I understand you correctly, you would accept a PR that made the individual lib-loading functions public, as long as they were all LGFunction functions named LUA_OSLIBNAME etcetera?

@cathalgarvey
Copy link
Contributor

And BTW, I'm aware of, and in awe of, the fact that this is just hobby stuff for you. Thanks so much for Gopher-Lua, it's amazing and really useful. :)

@yuin
Copy link
Owner

yuin commented Jan 16, 2016

So, if I understand you correctly, you would accept a PR that made the individual lib-loading functions public, as long as they were all LGFunction functions named LUA_OSLIBNAME etcetera?

  • Rewrite openXXX functions as an LGFunction
  • Create a new file : linit.go
    • Add constants to the linit.go, like TabLibName = "table"
    • Move the OpenLibs() to linit.go
    • Modify the OpenLibs() to use new constants and new openXXX functions the same way as linit.c.

@cathalgarvey
Copy link
Contributor

Mission accepted. :)

@milochristiansen - Sound like a solution for you also?

cathalgarvey added a commit to cathalgarvey/gopher-lua that referenced this issue Jan 19, 2016
@cathalgarvey
Copy link
Contributor

Hi @yuin,

I've made the changes as I understand them, and pushed them to the linit branch on my fork - when you get a chance could you review before I PR?

Thanks!

@yuin
Copy link
Owner

yuin commented Jan 19, 2016

Code review on the github should be made in PR. Please send a pull request.

@cathalgarvey
Copy link
Contributor

Done

cathalgarvey added a commit to cathalgarvey/gopher-lua that referenced this issue Jan 20, 2016
New private type, luaLib, to contain name and load function, same
idea as Lua5.1's luaL_Reg: http://pgl.yoyo.org/luai/i/luaL_Reg

luaLibs is now private, is now a slice of luaLibs.

Each module register function now returns the module table.

luaLibs are now executed by pushing onto the stack, pushing
the module name, and calling with args 1 and returns 0.

Some comments removed by request.
@yuin yuin closed this as completed in cbab8d2 Jan 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants