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

load / loadfile lua functions can load arbitrary bytecode #116

Closed
kyren opened this issue Feb 3, 2019 · 5 comments
Closed

load / loadfile lua functions can load arbitrary bytecode #116

kyren opened this issue Feb 3, 2019 · 5 comments

Comments

@kyren
Copy link
Contributor

kyren commented Feb 3, 2019

AIUI this can be used to cause the interpreter to exhibit UB.

load and loadfile need to be wrapped so that the mode flag does not do anything, but unlike pcall / xpcall wrapping, this really seems to break the API of those functions.

There has to be the ability to turn these wrappers off, just like there is the ability to force load the debug library, and similarly it will have to be behind an unsafe flag.

The load and loadfile wrappers shouldn't be too bad, and configuring them could be as simple as adding a flag to StdLib that disables wrapping load / loadfile, though that breaks the StdLib <=> 'library' equivalence.

Probably this has to be a new 0.x version bump as well 😞

@kyren
Copy link
Contributor Author

kyren commented Feb 3, 2019

Also package.loadlib allows a script to load a C library, which is also really unsafe.

Am I missing any more?

@kyren
Copy link
Contributor Author

kyren commented Feb 3, 2019

I wonder if dofile will execute bytecode?

@kyren
Copy link
Contributor Author

kyren commented Feb 3, 2019

Okay, here is a list of everything that I think is potentially unsafe, some of it may simply have to be not loaded at all. Currently only debug is guarded against:

  • dofile
  • load
  • loadfile
  • os.setlocale (thread unsafety?)
  • package.loadlib
  • os.date (maybe, might be fixed in 5.2, might be considered a bug)
  • debug (okay, technically some of debug might be safe, but is of questionable use by itself)
  • package.cpath (can be used to influence require to load C libraries, without package.cpath, none of the default searchers in package.searchers should be able to find and load a C library

@kyren
Copy link
Contributor Author

kyren commented Feb 3, 2019

It might be simpler to set package.cpath to "" and disallow package.* entirely, as package.cpath should not be allowed to be set at all. Without setting package.cpath to empty string, require becomes unsafe.

@jugglerchris
Copy link
Collaborator

I have spun out #237 (os.date) and #238 (os.setlocale) as I think they're less serious.
The others are now wrapped or removed by default, with InitFlags which can be disabled to allow the originals.

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

2 participants