SCRIPT tags and sandboxing discussion
dmarcuse opened this issue · 29 comments
Here we are
You are added to my repo, I already added the basic sandbox + execution code, really basic tho. @apemanzilla
Alright, so: We should use a whitelist for previously stated reasons (to prevent problems with future CC versions)
On top of that, we should make sure that all tables are hard copied, or use protected metatables like so:
Bad
local env = {}
env.colors = colors
Good
local env = {}
env.colors = setmetatable({}, {__index = colors, __metatable = "protected"})
This is to prevent scripts from modifying functions that may later be called by kristscape or CraftOS or other programs.
This is to prevent scripts from modifying functions that may later be called by kristscape or CraftOS or other programs.
Clever, didn't thought of that, but yeah that's what we should do
Also, to prevent breaking out of the sandbox, functions like setfenv
, and more dangerously, getfenv
should not be whitelisted.
I knew of that. I wouldn't put anything where theres no reason of adding it, like direct print()
An api like JS document would be something good to interact with the content
print
itself should be alright as long as the scripts are redirected to prevent modifying the top level windows.
Functions like term.native
should also be either overwritten with a 'safe' version or left out entirely for this reason.
print
was just an example, but it isn't needed, so leaving it out might be senseful so that you can't annoy the user by printing in e.g. the address bar
EDIT: Maybe replacing term with window.create(windowSpecsOfContentInKristscape) would be a good idea
print
obeys terminal redirection, so if the terminal is redirected properly, the only place print
could write to would be the viewpane.
then we could add print without making any website being able to fake something(e.g. by just printing in the address bar)
Also, maybe having a kristscape.request
function to allow scripts to request access to non-whitelisted APIs would be useful - requiring the user to allow or deny the request:
local fs = kristscape.request "fs"
if fs then
-- user allowed request
else
-- user denied request
end
I like how firewolf handles fs, it uses a download function. But yes a request function would be nice too.
I'm going for a more Chrome-style approach. Scripts are started with minimal permissions, requiring them to request access to the lower level stuff.
that is probably the best solution
I tried to add the mt thing, I'm not 100% sure how it works, so it would be nice if you could check https://github.com/Luca0208/kristscape/commit/b39d0deae0132ac4ab9c1f9a92ec049663624885
That should be OK. The __metatable
field is just there to prevent people getting access to the other metamethods.
Should the sandboxing stuff be moved to its own file?
Should the sandboxing stuff be moved to its own file?
Maybe, if he's online @BTCTaras might wanna give his opinion here
Ok, Taras is not online or can't respond right now, but the whitelist could go relatively large, so an external file would prevent that the ksml.lua file get's too big
Hold on, I'll get him online.
Nice, thanks!
I'm going to sleep now, 23:47 here, do you know what time zone Taras lives in? I am in CEST.
I had intended for the argument in the script tag to specify which interpreter to use (i.e. sandboxed lua would be [SCRIPT:LUA]
) and yes I do think it should be in a separate file
There's a problem when enabling term api for use and making a window. As soon as we create a window it will override everything already on the term
Try window.create(term.current(), x, y, w, h, false)
I actually tried it now, but the whole screen turns black
So we now have scripts 😄
what about global functions tho?
If we do 2 languages they should be similar so I reverted some of the progress to use systems like Inquire, but it doesn't work if someone could take a look at 766efd7
Now that this is further ahead than expected, we can go ahead and slate it for 1.0