bugFreeciv - Bugs: bug #15624, [RFC] scripting: Sandbox Lua...

 
 
Show feedback again

bug #15624: [RFC] scripting: Sandbox Lua scripts [Remove unsafe functionality]

Submitted by:  Engla <englabenny>
Submitted on:  Sun 14 Mar 2010 10:40:31 PM UTC  
 
Category: generalSeverity: 4 - Important
Priority: 7 - HighStatus: Fixed
Assigned to: Engla <englabenny>Open/Closed: Closed
Release: Operating System: None
Planned Release: 2.1.12, 2.2.1, 2.3.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 26 Mar 2010 12:14:04 PM UTC, comment #16:

The bug should now be fixed in all branches supporting scripting, from 2.1 to trunk.

Notice that the bug resolution was not to Sandbox Lua scripts, but to sanitize our whole Lua runtime; there should be no way to access the operating system from inside Lua.

Engla <englabenny>
Project MemberIn charge of this item.
Fri 26 Mar 2010 12:09:42 PM UTC, SVN revision 17159:

Make impossible to access operating system from Lua scripts

For security reasons, Lua scripts should not be able to read files or
run programs on the host computer; freeciv scenarios should only be
able to influence the state of the game, not the state of the server
process or computer (except through normal scenario events, such as
end of game).

For this reason, we do not load some standard lua libraries that allow
access to files or the operating system. We also disallow loading lua
libraries so that the script cannot go around this restriction.

This is the 2.1-branch version (Lua 5.0): we exclude the io library
('io' and 'os' modules) and blacklist functions dofile, loadfile and
require.

See gna bug #15624

(Browse SVN revision 17159)

Engla <englabenny>
Project MemberIn charge of this item.
Fri 26 Mar 2010 12:09:18 PM UTC, SVN revision 17158:

Make impossible to access operating system from Lua scripts

For security reasons, Lua scripts should not be able to read files or
run programs on the host computer; freeciv scenarios should only be
able to influence the state of the game, not the state of the server
process or computer (except through normal scenario events, such as
end of game).

For this reason, we do not load some standard lua libraries that allow
access to files or the operating system. We also disallow loading lua
libraries so that the script cannot go around this restriction.

This is the 2.2 and trunk version (Lua 5.1): we exclude the io
library, os library, package library, and blacklist functions dofile,
loadfile.

For Lua 5.1, the list of modules and functions we consider unsafe are:

"os",
"io",
"package",
"dofile",
"loadfile",
"module",
"require"

These are all unavailable by not being loaded or being explicitly blocked.

See gna bug #15624

(Browse SVN revision 17158)

Engla <englabenny>
Project MemberIn charge of this item.
Fri 26 Mar 2010 12:08:47 PM UTC, SVN revision 17157:

Make impossible to access operating system from Lua scripts

For security reasons, Lua scripts should not be able to read files or
run programs on the host computer; freeciv scenarios should only be
able to influence the state of the game, not the state of the server
process or computer (except through normal scenario events, such as
end of game).

For this reason, we do not load some standard lua libraries that allow
access to files or the operating system. We also disallow loading lua
libraries so that the script cannot go around this restriction.

This is the 2.2 and trunk version (Lua 5.1): we exclude the io
library, os library, package library, and blacklist functions dofile,
loadfile.

For Lua 5.1, the list of modules and functions we consider unsafe are:

"os",
"io",
"package",
"dofile",
"loadfile",
"module",
"require"

These are all unavailable by not being loaded or being explicitly blocked.

See gna bug #15624

(Browse SVN revision 17157)

Engla <englabenny>
Project MemberIn charge of this item.
Wed 24 Mar 2010 03:54:39 PM UTC, comment #12:

A very similar version, but in "pure C". The 2.2+trunk version of the patch has an added compiler warning if the unsafe functions information is not updated for a new Lua version. The 2.1 patch does not have this, since Lua 5.0 doesn't define a numeric version symbol.

Ideally, our lua script runtime should be configured in lua. Two reasons for pure C:

1. It's what freeciv coders understand.
2. Tolua is regrettably not so conductive for doing this since, among other things, errors (explicit by asserts or unintentional ones too) are silently ignored in embedded code in api.pkg.

(file #8638, file #8639)

Engla <englabenny>
Project MemberIn charge of this item.
Wed 24 Mar 2010 12:31:37 PM UTC, comment #11:

Testcase Scenario. This Scenario will run a couple of asserts to make sure none of the Lua 5.1 unsafe functionality is available through their standard ways. A successful run will not show any lua errors or output at all neither at scenario load time or at the start of the first turn.

From what we know now, this savefile assures that a freeciv lua script can not access files or run programs.

(file #8637)

Engla <englabenny>
Project MemberIn charge of this item.
Wed 24 Mar 2010 12:22:07 PM UTC, comment #10:

This solution is easy and less complex/less lines of code, which makes it easy to choose. This solution is also very easily applied to the 2.1 branch. (Less code than in 2.2 or later due to how Lua 5.1 removed the public functions to load libraries one by one).

------

Subject: [PATCH] Make impossible to access operating system from Lua scripts

For security reasons, Lua scripts should not be able to read files or
run programs on the host computer; freeciv scenarios should only be
able to influence the state of the game, not the state of the server
process or computer (except through normal scenario events, such as
end of game).

For this reason, we do not load some standard lua libraries that allow
access to files or the operating system. We also disallow loading lua
libraries so that the script cannot go around this restriction.

This is the 2.2 and trunk version (Lua 5.1): we exclude the io
library, os library, and blacklist functions dofile, loadfile.

For Lua 5.1, the list of modules and functions we consider unsafe are:

"os",
"io",
"package",
"dofile",
"loadfile",
"loadlib",
"module",
"require"

These are all unavailable by not being loaded or being explicitly blocked.

(file #8635, file #8636)

Engla <englabenny>
Project MemberIn charge of this item.
Wed 24 Mar 2010 12:17:30 PM UTC, comment #9:

The first proposed solution, providing an unbreakable sandbox, is not the best solution. The problem with the first patch is that tolua exposes all the userdata types' metatables (as the names of each type), and the script can then overwrite special functions, such as the '__gc' slot, and we can't control when that function is called.

It is possible to make all the metatables read-only but it gets more complicated, and we haven't gone trough all of tolua yet.

It is easier to just block the io and os libraries.

Engla <englabenny>
Project MemberIn charge of this item.
Sat 20 Mar 2010 11:48:44 AM UTC, comment #8:

Be sure that by saying that security is hard, I don't mean that I can do it better than you -- on the contrary, if we work together, this will be much better. What I meant was: don't trust me yet, it's not so easy.

Engla <englabenny>
Project MemberIn charge of this item.
Sat 20 Mar 2010 11:44:42 AM UTC, comment #7:

I conclude, that I will leave this to you. Hopping you will return fast.

pepeto <pepeto>
Project Member
Sat 20 Mar 2010 11:30:37 AM UTC, comment #6:

I agree that the security issue is very important. The caveat is that just because we think it works doesn't mean that it does, security is hard, especially with a runtime that we don't know so well :-)

Luckily, the lua runtime is very small. We can investigate not even loading the os and io libraries, however that adds more code (and can only be done with lua calls, not C calls in 5.1?).

What is important now is that all script code is executed inside our setfenv environment, and that none of the remaining functions can give the script access to the outside global environment (like getfenv, load, require, module etc could do).

I have not investigated if there are any holes in tolua. The tolua module (providing tolua.type that we need inside the restricted environment) has also not been investigated.

Notice that we may replace all modules (string, table, math, coroutine (should it be included?) and tolua) by tables that contain just the functions we want, if we need to block out any of them.

Engla <englabenny>
Project MemberIn charge of this item.
Sat 20 Mar 2010 11:10:25 AM UTC, comment #5:

> Ok, no not really. Feel free to work on this. I will type up documentation later.


I thought it was ready. It's a big security problem. Maybe documentation could be done later.

> What will happen to 2.1? It looks like it is still using Lua 5.0
> while 2.2 is using 5.1.


It will stay like this.

pepeto <pepeto>
Project Member
Sat 20 Mar 2010 11:03:04 AM UTC, comment #4:

Ok, no not really. Feel free to work on this. I will type up documentation later. What will happen to 2.1? It looks like it is still using Lua 5.0 while 2.2 is using 5.1.

Engla <englabenny>
Project MemberIn charge of this item.
Thu 18 Mar 2010 05:14:35 PM UTC, comment #3:

Will you be able to work on it and commit it in a near futur?

pepeto <pepeto>
Project Member
Thu 18 Mar 2010 11:12:53 AM UTC, comment #2:

Can I assign this bug to me?

Engla <englabenny>
Project MemberIn charge of this item.
Sun 14 Mar 2010 10:50:27 PM UTC, comment #1:

Testcase scenario. This file does not verify the security of the solution, it only demonstrates the problem.

./ser -f OSAccess.sav

before:

1: Hello esteemed freeciv user.
1: Let's check uname -a
Linux ulrik-ibook 2.6.32-trunk-powerpc #1 Mon Jan 11 03:50:43 UTC 2010 ppc GNU/Linux
1: Maybe I can read your files
1: root:x:0:0:root:/root:/bin/bash
daemon:x:1:1:daemon:/usr/sbin:/bin/sh
bin:x:2:2:bin:/bin:/bin/sh
...

after:

1: Hello esteemed freeciv user.
1: Let's check uname -a
1: lua error:
[string "script.code"]:4: attempt to index global 'os' (a nil value)
stack traceback:
[string "script.code"]:4: in main chunk

1:
2: error_log("Hello esteemed freeciv user.")
3: error_log("Let's check uname -a")
--> 4: os.execute("uname -a")
5: error_log("Maybe I can read your files")
6: error_log(io.open("/etc/passwd", "r"):read("*a"))

We must be sure that one can't retrieve the modules io, os again (which is why we block require, module, package etc.).

(file #8532)

Engla <englabenny>
Project MemberIn charge of this item.
Sun 14 Mar 2010 10:40:31 PM UTC, original submission:

Code executed in our Lua runtime has by default access to all builtin
lua functions and modules. These include functions to load lua files
or access the operating system.

As an example, a ruleset or scenario script could execute arbitrary
shell scripts (for example the 'uptime' program to use a harmless
example) using: os.execute("uptime"). Additionally the builtin module
io allows lua code to open and read/write files.

If we can, we should make freeciv rock-solid safe w.r.t scenario
scripts, they should be simple data, without security implications.
Otherwise a server administrator must scrutinize any custom ruleset
and scenarios before installing them. And users could experience
viruses in the form of freeciv scenarios or savegames.

Lua provides a method called "setfenv" that allows the caller to set
the environment a called function executes in. We set up a restricted
environment and execute ruleset/scenario code only inside this. In the
code, this restricted execution is carried out inside
script.c:script_call (which is now the only entry point for user
code).

The setup of the restricted environment uses a whitelist of builtin
symbols (functions, values and modules) that we allow in the scripting
environment, defined in api.pkg, where we also have a comment:

We want to assure that
1) The script has no access to the operating system
(loadfile, os module, io module).
2) The script can not modify modules that freeciv's script runtime
uses, for example by diverting error handling routines or similar.
3) The script can not break out of the sandbox.

I have used this community resource as reference when picking builtins
to whitelist:

http://lua-users.org/wiki/SandBoxes

Notice however that a normal freeciv script needs next to no builtins.
We don't forsee needing class and inheritance programming, so much of
lua's power can be turned off. The whitelist of builtins is thus
small.

The sandbox construction assumes that all parts of our game api are
safe.

Engla <englabenny>
Project MemberIn charge of this item.

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by pepeto (Updated the item)
  • -unavailable- added by englabenny (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 25 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 18 Apr 2010 10:24:03 PM UTCenglabennySummary[RFC] scripting: Sandbox Lua scripts=>[RFC] scripting: Sandbox Lua scripts [Remove unsafe functionality]
    Sun 28 Mar 2010 10:56:25 AM UTCenglabennyDependencies-=>bugs #15725 is dependent
    Fri 26 Mar 2010 12:29:24 PM UTCenglabennyStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 26 Mar 2010 12:14:04 PM UTCenglabennyRelease=>2.1
    Fri 26 Mar 2010 12:14:03 PM UTCenglabennyStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Thu 25 Mar 2010 06:45:30 PM UTCenglabennyDependencies-=>bugs #15644 is dependent
    Wed 24 Mar 2010 03:54:39 PM UTCenglabennyAttached File-=>Added 0001-Make-impossible-to-access-operating-system-from-Lua-.patch, #8639
    Wed 24 Mar 2010 03:54:38 PM UTCenglabennyAttached File-=>Added 0001-Make-impossible-to-access-operating-system-2_1.patch, #8638
    Wed 24 Mar 2010 12:37:08 PM UTCenglabennyDependenciesRemoved dependancy to patch #1534=>-
    Wed 24 Mar 2010 12:31:37 PM UTCenglabennyAttached File-=>Added ScriptSecurityTest.sav, #8637
    Wed 24 Mar 2010 12:22:07 PM UTCenglabennyAttached File-=>Added 0001-Make-impossible-to-access-operating-system-from-Lua-.patch, #8635
      Attached File-=>Added 0001-Make-impossible-to-access-operating-system-2_1.patch, #8636
      StatusIn Progress=>Ready For Test
    Sat 20 Mar 2010 11:44:42 AM UTCpepetoStatusReady For Test=>In Progress
      Assigned topepeto=>englabenny
    Thu 18 Mar 2010 11:00:31 AM UTCenglabennyCarbon-Copy-=>Added -unavailable-
    Wed 17 Mar 2010 11:02:25 AM UTCpepetoSeverity3 - Normal=>4 - Important
      Priority1 - Later=>7 - High
      Planned Release2.2.1, 2.3.0=>2.1.12, 2.2.1, 2.3.0
    Wed 17 Mar 2010 09:08:48 AM UTCpepetoStatusNone=>Ready For Test
      Assigned toNone=>pepeto
      Planned Release=>2.2.1, 2.3.0
    Sun 14 Mar 2010 10:50:27 PM UTCenglabennyAttached File-=>Added OSAccess.sav, #8532
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup