fixing the "last arg is the input file" assumption - #17
Open
nzatsepina wants to merge 1 commit into
Open
Conversation
ExpandPathsInArgs() rewrote nargv[argc-1] to an absolute path, and ApplyPolicy() whitelisted that same token, whenever it did not begin with '-'. Every other file Ghostscript opens was therefore missing from the sandbox policy: multiple inputs, -ffile, and files given after --/-+/-@ were denied (exit 100, no output) although unsandboxed gsc.exe runs all of them. This is the case the TODO in the header and the FIXME beside the code already flag for -f. ClassifyInputArgs() now walks the arguments the way psi/imainarg.c does and marks the tokens Ghostscript will open as files; those are made absolute and whitelisted. Four paths are deliberately not modelled, and are listed above the function. A token that does not exist relative to the working directory is left untouched, because Ghostscript may resolve it on its library search path. This narrows the policy as well as widening it: the last argument is no longer whitelisted merely for being last, so an -o or -I operand sitting there loses a grant it used to receive by accident. The -sOutputFile= rewrite in the same loop now range-checks GetFullPathName(): when the buffer is too small the documented return is the required size rather than the length written, so a bare "> 0" test read that failure as success. Fixes PaperCutSoftware#3.
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
1 participant
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.
ExpandPathsInArgs() rewrote nargv[argc-1] to an absolute path, and ApplyPolicy() whitelisted that same token, whenever it did not begin with '-'. Every other file Ghostscript opens was therefore missing from the sandbox policy: multiple inputs, -ffile, and files given after --/-+/-@ were denied (exit 100, no output) although unsandboxed gsc.exe runs all of them. This is the case the TODO in the header and the FIXME beside the code already flag for -f.
ClassifyInputArgs() now walks the arguments the way psi/imainarg.c does and marks the tokens Ghostscript will open as files; those are made absolute and whitelisted. Four paths are deliberately not modelled, and are listed above the function. A token that does not exist relative to the working directory is left untouched, because Ghostscript may resolve it on its library search path.
This narrows the policy as well as widening it: the last argument is no longer whitelisted merely for being last, so an -o or -I operand sitting there loses a grant it used to receive by accident.
The -sOutputFile= rewrite in the same loop now range-checks GetFullPathName(): when the buffer is too small the documented return is the required size rather than the length written, so a bare "> 0" test read that failure as success.
Fixes #3.