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

Restrict Lua standard library #1317

Merged
merged 15 commits into from
Jul 5, 2012

Conversation

johnbartholomew
Copy link
Contributor

Pioneer has three different Lua contexts: the module system, LMR, and the custom systems loader. They each provide different things to their scripts, but until fairly recently they have exposed all of the Lua standard library, which includes various bits of functionality that makes scripts potentially dangerous or difficult to write correctly.

This PR defines a restricted form of the Lua standard library, and changes all three contexts to use that form. Each context still adds various extensions on top of this base.

Things this excludes from the Lua standard library:

  • The os and io packages: These are very dangerous since they let scripts read and write to any file on the system (if the user has permission), and execute arbitrary commands through os.execute.
  • math.random and math.randomseed: These behave differently on different platforms, and have uncertain statistical properties. Random number generators also tend to be difficult to use effectively if you have any requirements for isolation, repeatability, determinism and so on. These functions are the reason that the Lanner on the main menu looks different on different platforms.
  • dofile, loadfile, require and the package package: These bypass Pioneer's filesystem abstraction, so they don't work properly with installable mods or have platform specific behaviour. They also allow access to Lua scripts anywhere on the system (probably ok, but maybe not). Finally, the package package allows loading of native dynamic libraries, which is potentially dangerous.

Things added to the Lua standard library:

  • math.deg2rad is provided as an alias for math.rad, because deg2rad is a better name.
  • util.hash_random is provided as an alternative to horrible hacks involving math.randomseed and math.random. The module API provides real RNG support. hash_random is just a useful way to make a deterministic choice from some arbitrary input data.

TODO

  • Work out what to do about a replacement for require. Decision: Post-poned for a separate pull request, but most likely a function that works fairly similarly to Lua's standard require function (just hooked up to the file system to work properly with mods). require isn't actually used right now, because all Lua scripts are loaded recursively anyway.
  • Document util.hash_random properly, or move or remove it if it seems like a bad idea. Done
  • Re-instate some form of core dump in pidebug.lua. It has been disabled for now because it uses the io library, which is no longer available in that context. Decision: The core dump isn't actually useful in practice, so pidebug has just been completely scrapped for now, with the hope that someone will come in and implement better debugging tools in the future.

John Bartholomew added 12 commits June 28, 2012 00:20
hash_random can be used to robustly make a selection based on some
input data. For a lot of purposes, you want this and not a random
number generator, because RNGs are hard to use nicely, and inherently
tie together any an all code that touches them.

I make no guarantees that hash_random will not change its output in
future versions, but within a single version its output should be
reliable across platforms, and because it's a hash you can explicitly
and carefully control what inputs you wish to affect it.
this is still a crappy thing to do, and the functions still set global
state instead of returning a value, but it removes the dependency on
math.random/math.randomseed.
This is actually kinda dumb, because the custom systems use almost nothing
from the Lua standard library, so this actually increases the amount of
stuff avaiable in the context. But... consistency... :-/
@robn
Copy link
Member

robn commented Jun 29, 2012

I like the approach and the rationale.

hash_random seems useful, certainly much better than the atrocities currently being committed in selector.lua. I'm not sure of its usefulness outside that and so given the impending removal of LMR, I wonder if its worth it. I also wonder if it belongs on math or under some util or similar namespace, I don't think I care strongly about either though, so don't feel any particular need to argue. It is a great improvement over the current situation which cannot be a bad thing.

For require I think the concept of the standard Lua function of the same name is good, even if it doesn't quite fit Pioneer. A simple implementation might be that for require("foo"), the global table foo is checked. If it exists, nothing happens. If it doesn't exist, data/libs/foo.lua is executed, and is expected to define the global table foo.

There is room for what might be termed abuses (libraries doing more than just defining the table, something else defining the table and thus stopping the thing loading), but its a very nice simple implementation. And it will continue to do the right thing if we start sandboxing modules in the future by giving each its own global table. In that case, require will also populate the module's global table.

@johnbartholomew
Copy link
Contributor Author

I think hash_random is actually very useful. If you need deterministic output (which we often do) it's much easier to be confident of getting it by using a hash based method than using an RNG. Sure, you can create a seeded RNG and pull one number from it, but that's just a more convoluted method of getting a hash (*).

If you look at the sysgen code, it uses an RNG seeded with the system path and universe seed, but because the samples taken from that RNG are interleaved with the complicated sysgen algorithm it's basically impossible to make controlled changes to sysgen: that is, if you change one bit of it you'll probably end up changing everything. That wouldn't be the case if hash based code were used. Instead of creating one RNG and then pulling numbers from it as needed, each number can come from a hash of (universe seed + system seed + key), where the key is, say, the name of the field you're generating.

RNGs do become useful when you need a whole sequence of values and you want to be confident of the statistical properties of the sequence as a whole. For example, generating sampling points for stochastic sampling. You could do this as a load of separate hashes of (seed + counter), but that's slower and it's harder to be confident of the distribution because hashes have somewhat different design goals to RNGs.

You may be right that hash_random should go in util or something, rather than the math table.

(*) In fact, if you have a bunch of related seeds (e.g., system paths, where systems which are close together in space are close together in bit pattern), and you want to generate a random number for each one, and you want a uniform distribution of output values, then a real hash function is probably a much better choice than re-seeding an RNG for each input and pulling one number from it.

@robn
Copy link
Member

robn commented Jul 1, 2012

If you look at the sysgen code, it uses an RNG seeded with the system path and universe seed, but because the samples taken from that RNG are interleaved with the complicated sysgen algorithm it's basically impossible to make controlled changes to sysgen: that is, if you change one bit of it you'll probably end up changing everything.

That's actually a really good point. For NameGen I actually created a separate RNG to pass out to Lua just so it wouldn't screw things upl This would make things much easier. Case made - I'm sold! :)

John Bartholomew added 3 commits July 4, 2012 22:23
More importantly, the 'io' library is being disabled for safety reasons,
so there's no way of emitting the core dump any more. pidebug can be
brought back if/when someone adds a better debug output system and exposes
it to Lua.
@robn robn merged commit 11ae9ec into pioneerspacesim:master Jul 5, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants