Security issue with package library in Lua 5.1

MUSHclient 3.80 moves to Lua version 5.1. This has many improvements as described here:

http://www.gammon.com.au/forum/?id=7322

However a potential security problem is that the "loadlib" function has been moved from the base library to the package library, where the package library did not exist in earlier versions of Lua.

My concerns with loadlib are that it can be used to load any DLL into the Lua address space (and thus the MUSHclient address space).

This is certainly useful for various cases that have been documented on this web site, including:

• Use of the MySQL database library
  
• Use of the ODBC database library
  
• Use of the LuaSocket library
  
• Use of the Lua COM library
  
• Use of the Logitech G15 keyboard library
  
• Use of the encryption library

However it also gives malicious coders a tool to inject viruses or trojan horse programs onto your PC. The general technique would be this:

• Write a virus or trojan horse as a DLL, which will have access to all of the Windows system libraries, including file IO, access to the Internet and so on
  
• Write a plugin which claims to do something useful (eg. a mapper)
  
• Claim that the plugin "must have" the DLL in order to operate properly
  
• Distribute both from a web site operated by the malicious coder, and encourage their use
  
• Once installed the DLL could cause various sorts of havoc

In the past MUSHclient has had a "Lua sandbox" which has removed access to the loadlib function, like this:



There are also comments in the standbox showing how that it can be enabled for trusted plugins.

However the problem with Lua 5.1 is that the loadlib function is no longer in the global environment, and thus setting loadlib to nil does nothing, as it is already nil.

Loadlib is now inside the "package" library (package.loadlib).

To work around this issue, MUSHclient version 3.80 has a new checkbox on the Global Configuration Dialog (Lua sandbox page) called "Enable Package Library" which defaults to off. With this unchecked, MUSHclient removes the entire package library from the Lua address space while loading the Lua engine.

To preserve the security of your system, you are encouraged to leave this unchecked.

If you need to use loadlib (eg. to access a database) then you should edit the sandbox code and change:



You should probably follow the guidelines in the comments in the sandbox, and only allow the use of loadlib for trusted plugins.



[EDIT]

This is no longer the case, exactly, see below.

'require' function disabled by default

The 'require' function in the base library also has much the same problem as package.loadlib.

It now loads DLLs as part of its "search for a package" functionality, and is thus potentially more dangerous than the old 'require' function, which only loaded Lua code.

Thus there is now another checkbox on the Lua sandbox preferences dialog, defaulting to off, that you need to check if you really need to use the 'require' function.

[EDIT]

This is no longer the case, see below.

Nooooo! I need require! There's no way to specify that it can only be used for Lua file, not DLL's?

You only have to uncheck the box once. Or are you worried about distributing your plugins?

It can be done, I may have to rethink my strategy here.

For example, in this test:




Is your argument that:

• You need 'require' for plugins you distribute
  
• You only need to load Lua code (for modularity reasons), but not DLLs
  
• You don't want end-users to have to fiddle with settings

To an extent all of the above, but by far the strongest argument is the end user one.

I only use require to load Lua code, no DLLs at the moment, and I expect it to stay that way.

I load Lua code for modularity reasons. Obviously I could get around this by simply making it all one file. However, 15000 lines in one file could get messy :( Still this is workable.

I would prefer not to have end users having to mess with the settings to get it to work. However, thinking about this, one little checkbox is probably with reasonable realms, and I can always flame them if they dont read the docs before using it :)

So then, the end user issue seems not as great as the fact that I'd have to wade through those 15000 lines to find anything.

-----

Looking at the error message stack trace, it isn't complaining specifically about the DLL loader or the all-in-one loader being nil. What are the results if you disable ONLY the DLL loader or the all-in-one loader?

As a last-ditch solution if nothing can be worked out, what do you think about creating a MUSHclient function called LuaAskRequire or something? When called by a plugin it would pop-up a box saying something like, 'This plugin requires DLL loading capabilities which are not enabled by default. If you enable this, the plugin could execute malicious code! Enable this only if you trust the plugin source...' and giving the option to enable it on the spot?

-Tsunami

I assumed you would say that, and have been working on a different solution. As both require and package.loadlib can load DLLs, it seems a more rounded solution is one that specifically disables DLL loading in a single checkbox.

I have reworded the checkbox to "Allow DLLs to be loaded".

If unchecked it does the following:

• package.loadlib = nil --> disable direct DLL loading
  
• package.loaders [3] = nil --> disable DLL loader
  
• package.loaders [4] = nil --> disable all-in-one loader

Now you should be able to simply "require" stuff and have it work.

You may want to look at the default directories it uses for require and rejig your directory structure to conform to it.



Each loader outputs an error message, so the more loaders you disable the less messages.

Hopefully this will address your requirements. :)

You might want to seriously look at the new "module" feature in Lua 5.1. It takes a bit of getting used to, but is designed to be used in conjunction with "require". Here is a small example. Say I have a file foo.lua, looking like this:

---

Now having saved that file to disk as foo.lua, I can now require its use:



After doing that I can see what the global variable foo is set to:



There are a number of interesting behaviours exposed by this test:

• Although the word "foo" does not appear anywhere inside the "foo.lua" file, it can find its own name by referring to _NAME.
  
• Only non-local variables are exposed to the outside environment (in my case foo.bar is the only exposed function).
  
• Inside the module, it can make internal references to other parts of the module without having to qualify the names (eg. it can call function f, which is really foo.f).
  
• It has read access to all global variables (via the package.seeall function). However if you didn't want that you could explicitly nominate which global functions you want, before the "module" line, eg.
  
  

  
  local print = print --> I need the print function
  local string = string --> I need the string library
  

  
  
• If you forget to put "local" in front of a variable it is still local to the "foo" table. eg.
  
  

  
  d = 99  --> foo.d is 99, not global d
  

  
  
  This means you cannot "pollute" the global namespace accidentally, and thus modules are protected from getting at each other's variables.

Basically this is a great mechanism for writing modular code.

Correct me if I'm wrong, but that still needs require... neh?

Sorry, only saw latest post, not one before.

Yes, it does. It works in conjunction with "require".

First point is that "require" will now be available by default in MUSHclient 3.80, however the DLL part won't be working. That is, you can require Lua code only (unless you check the checkbox).

The functions "require" and "module" are supposed to complement each other.

A module would be in a separate file, something like this:

---

Assuming we put all the above into "chatsystem.lua", all someone that wants to use it has to do is:



Any non-local variables inside the chatsystem.lua file in this example are "exported" into the newly created chatsystem table. Thus you might then do:




Effectively it makes it easy to keep your code modular.

The "require" statement in Lua 5.0 basically just loaded code by walking a few paths of possible locations it might be in. This extra stuff about making module tables etc. is new to Lua 5.1, and needs the support of the "module" function to fully implement it.

The Lua book explains it quite well, I don't know so much about the online documentation.

Looks a lot like Python modules.

In fact, Python does that automatically when loading other Python scripts into one script.

Ie

import re
re.blah

hehe

I was going to ask why it took getting used to when it's exactly like namespaces in C++, but now I can't remember if MUSH is coded in C or C++... *sigh*

Actually Lua modules are not "exactly" like namespaces in C++. They do scope package contents into namespaces, yes, but there are several other things going on.

For one when you are declaring a module, you lose access to the global table (it uses the module table as the new "global" instead). To get around this you can either switch the environment to use the global one, or more simply but also more verbose, import things before declaring the module, e.g.:

This creates local copies fo the 'io', 'string', and 'table' tables.

Furthermore, you can determine at runtime whether or not a specific module has been loaded with 'require', because of how the package management works -- it populates various tables with the loaded-packages information.

Basically modules behave like C++ namespaces, but with some extras, and some gotchas. :)

Quite right. You can access existing local variables like this:

module ("my_mod", package.seeall)

This adds an __index field to the current environment, so you can see existing variables, but attempts to create new ones are still in the package "namespace".

Actually in Lua it is really an "environment".

It is coded in C++. It makes string management much easier, as well as having lots of other advantages.