-
Notifications
You must be signed in to change notification settings - Fork 18
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
mueval changes currentDir without resetting #17
Comments
But the work-around cannot be applied in a concurrent setting (several muevals forkIO'ed) since the "current directory" is a property of the OS process, of which there's only one? |
You shouldn't be depending on mueval to be doing anything at the OS level. A mueval invocation is toxic waste to be thrown away as quickly as possible. (Ideally, one would run it in a new VM entirely, but I never got as far as considering how to make that workable.) It could be frozen, it could be in the middle of being corrupted, it could have been crippled by a partial attack... If you are depending mueval to read, parse, & execute untrusted evil Internet-controlled inputs and are relying on it to do anything at all after having done that such as changing environment state, that is wrong and a potential source of vulnerabilities. I don't know about concurrent settings, although forking new processes instead of green threads seems like the obvious thing to do? |
Yes I am taking that route now (evaluation in a separate process inside chroot). Your warnings are entierly understood, although it's mitigated somewhat since I am running code only after the AST matches a given pattern (it's for "fill the holes" programming exercises). The pattern also prescribes imports and pragmas, so students cannot use For reference, this "concurrent setting" issue only surfaced because we were switching our backend service from CGI (one process per request) to servant (just one OS process). |
When reading from a file, mueval changes the current directory and never resets it?
https://github.com/gwern/mueval/blob/master/Mueval/Interpreter.hs#L109
I think this should be documented - or changed. I am using this work-around:
https://gitlab.imn.htwk-leipzig.de/autotool/all0/blob/master/collection/src/Haskell/Blueprint/Central.hs#L120
The text was updated successfully, but these errors were encountered: