Apply the portrayal script restriction to package.loaded as well - #47
Open
SemanticWave-Hoyeon wants to merge 1 commit into
Open
SemanticWave-Hoyeon wants to merge 1 commit into
SemanticWave-Hoyeon wants to merge 1 commit into
Conversation
Portrayal catalogues arrive with an exchange set and are executed by this interpreter, so the standard library is kept away from them. The existing block did that by assigning nil to a few globals, which does not reach far enough: luaL_openlibs() also records every module it opens in package.loaded, and that table was left untouched, so require() returned the same module the global had been cleared of. On the LuaJIT runtime this project links against, the ffi module is reachable the same way and was not covered at all. The restriction now clears each module from _G, package.loaded and package.preload, drops the native module loaders along with package.cpath and package.loadlib, and removes the chunk loading entry points. Pure-Lua require() is left working, since rule files use it to pull in sibling rules (require 'PortrayalModel', require 'RESTRN01', and so on), and rawget is left alone because the rules use it heavily. The result of the chunk was previously discarded, so a restriction that failed to apply would have gone unnoticed while callers kept assuming it held. It is now checked, and a second chunk verifies that the modules and loaders are genuinely unreachable before any catalogue runs. Either failure aborts construction rather than continuing unrestricted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Portrayal catalogues arrive with an exchange set and are executed by this interpreter, so the constructor already tried to keep the standard library away from them by assigning
nilto a few globals.That does not reach far enough.
luaL_openlibs()also records every module it opens inpackage.loaded, and that table was left untouched, sorequire()returned the same module the global had just been cleared of. Clearing the global changes what the name resolves to; it does not remove the module.The same applies to the
ffimodule on the LuaJIT runtime this project links against (extlibs/Luais LuaJIT 2.1.0-beta3, despite whatreadme.txtsays), and that module was not covered by the original list at all.The change
The restriction now clears each module from
_G,package.loadedandpackage.preload, drops the native module loaders together withpackage.cpathandpackage.loadlib, and removes the chunk loading entry points.Two things are deliberately left alone:
require()keeps working, because rule files rely on it to pull in sibling rules (require 'PortrayalModel',require 'RESTRN01', and so on). Only the native loaders are removed, so module resolution still finds.luafiles and no longer finds anything else.rawgetstays, because the rule files use it heavily (35 call sites across the shipped catalogues).rawsetis cleared, as before, and is unused by them.The result of the chunk was previously discarded. A restriction that failed to apply would have gone unnoticed while the rest of the code kept assuming it held, which is the part that seemed most worth fixing. It is now checked, and a second chunk verifies that the modules and loaders are genuinely unreachable before any catalogue runs. Either failure closes the state and aborts construction rather than continuing unrestricted.
That last point is a behaviour change worth flagging:
lua_session's constructor can now throw. It only does so if the restriction did not take effect, which should not happen, but failing closed seemed the right default for something whose whole job is to hold. Say the word if you would rather it logged and continued.Verification
Checked against LuaJIT 2.1, matching the vendored runtime. Before the change,
require('os'),package.loaded.os,require('io'),require('debug')andrequire('ffi')all still resolve after the existing block runs. After it, each of those is refused, the chunk loaders are gone, and the verification chunk passes. Arequireof a plain.luamodule still loads normally, andrawgetis intact.No
.luafile is touched by this PR, and nothing underextlibs/is modified. As with the companion parser PR, I could not build the full MSVC/MFC project here, so this has not been compiled in place or run through the application.