Mercurial > hg
changeset 35170:c9740b69b9b7 stable
dispatch: add HGPLAIN=+strictflags to restrict early parsing of global options
If this feature is enabled, early options are parsed using the global options
table. As the parser stops processing options when non/unknown option is
encountered, it won't mistakenly take an option value as a new early option.
Still "--" can be injected to terminate the parsing (e.g. "hg -R -- log"), I
think it's unlikely to lead to an RCE.
To minimize a risk of this change, new fancyopts.earlygetopt() path is enabled
only when +strictflags is set. Also the strict parser doesn't support '--repo',
a short for '--repository' yet. This limitation will be removed later.
As this feature is backward incompatible, I decided to add a new opt-in
mechanism to HGPLAIN. I'm not pretty sure if this is the right choice, but
I'm thinking of adding +feature/-feature syntax to HGPLAIN. Alternatively,
we could add a new environment variable. Any bikeshedding is welcome.
Note that HGPLAIN=+strictflags doesn't work correctly in chg session since
command arguments are pre-processed in C. This wouldn't be easily fixed.
| author | Yuya Nishihara <yuya@tcha.org> |
|---|---|
| date | Thu, 23 Nov 2017 22:17:03 +0900 |
| parents | 898c6f812a51 |
| children | b85962350bb3 |
| files | mercurial/chgserver.py mercurial/dispatch.py mercurial/help/environment.txt mercurial/help/scripting.txt mercurial/ui.py tests/test-commandserver.t tests/test-dispatch.t |
| diffstat | 7 files changed, 127 insertions(+), 8 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/chgserver.py Thu Nov 23 22:04:53 2017 +0900 +++ b/mercurial/chgserver.py Thu Nov 23 22:17:03 2017 +0900 @@ -220,8 +220,17 @@ newui._csystem = srcui._csystem # command line args - args = args[:] - dispatch._parseconfig(newui, dispatch._earlygetopt(['--config'], args)) + options = {} + if srcui.plain('strictflags'): + options.update(dispatch._earlyparseopts(args)) + else: + args = args[:] + options['config'] = dispatch._earlygetopt(['--config'], args) + cwds = dispatch._earlygetopt(['--cwd'], args) + options['cwd'] = cwds and cwds[-1] or '' + rpath = dispatch._earlygetopt(["-R", "--repository", "--repo"], args) + options['repository'] = rpath and rpath[-1] or '' + dispatch._parseconfig(newui, options['config']) # stolen from tortoisehg.util.copydynamicconfig() for section, name, value in srcui.walkconfig(): @@ -232,10 +241,9 @@ newui.setconfig(section, name, value, source) # load wd and repo config, copied from dispatch.py - cwds = dispatch._earlygetopt(['--cwd'], args) - cwd = cwds and os.path.realpath(cwds[-1]) or None - rpath = dispatch._earlygetopt(["-R", "--repository", "--repo"], args) - rpath = rpath and rpath[-1] or '' + cwd = options['cwd'] + cwd = cwd and os.path.realpath(cwd) or None + rpath = options['repository'] path, newlui = dispatch._getlocal(newui, rpath, wd=cwd) return (newui, newlui)
--- a/mercurial/dispatch.py Thu Nov 23 22:04:53 2017 +0900 +++ b/mercurial/dispatch.py Thu Nov 23 22:17:03 2017 +0900 @@ -150,6 +150,8 @@ try: if not req.ui: req.ui = uimod.ui.load() + if req.ui.plain('strictflags'): + req.earlyoptions.update(_earlyparseopts(req.args)) if _earlyreqoptbool(req, 'traceback', ['--traceback']): req.ui.setconfig('ui', 'traceback', 'on', '--traceback') @@ -644,6 +646,12 @@ return configs +def _earlyparseopts(args): + options = {} + fancyopts.fancyopts(args, commands.globalopts, options, + gnu=False, early=True) + return options + def _earlygetopt(aliases, args, strip=True): """Return list of values for an option (or aliases). @@ -732,12 +740,16 @@ def _earlyreqopt(req, name, aliases): """Peek a list option without using a full options table""" + if req.ui.plain('strictflags'): + return req.earlyoptions[name] values = _earlygetopt(aliases, req.args, strip=False) req.earlyoptions[name] = values return values def _earlyreqoptstr(req, name, aliases): """Peek a string option without using a full options table""" + if req.ui.plain('strictflags'): + return req.earlyoptions[name] value = (_earlygetopt(aliases, req.args, strip=False) or [''])[-1] req.earlyoptions[name] = value return value @@ -745,13 +757,15 @@ def _earlyreqoptbool(req, name, aliases): """Peek a boolean option without using a full options table - >>> req = request([b'x', b'--debugger']) + >>> req = request([b'x', b'--debugger'], uimod.ui()) >>> _earlyreqoptbool(req, b'debugger', [b'--debugger']) True - >>> req = request([b'x', b'--', b'--debugger']) + >>> req = request([b'x', b'--', b'--debugger'], uimod.ui()) >>> _earlyreqoptbool(req, b'debugger', [b'--debugger']) """ + if req.ui.plain('strictflags'): + return req.earlyoptions[name] try: argcount = req.args.index("--") except ValueError:
--- a/mercurial/help/environment.txt Thu Nov 23 22:04:53 2017 +0900 +++ b/mercurial/help/environment.txt Thu Nov 23 22:17:03 2017 +0900 @@ -56,9 +56,17 @@ localization. This can be useful when scripting against Mercurial in the face of existing user configuration. + In addition to the features disabled by ``HGPLAIN=``, the following + values can be specified to adjust behavior: + + ``+strictflags`` + Restrict parsing of command line flags. + Equivalent options set via command line flags or environment variables are not overridden. + See :hg:`help scripting` for details. + HGPLAINEXCEPT This is a comma-separated list of features to preserve when HGPLAIN is enabled. Currently the following values are supported:
--- a/mercurial/help/scripting.txt Thu Nov 23 22:04:53 2017 +0900 +++ b/mercurial/help/scripting.txt Thu Nov 23 22:17:03 2017 +0900 @@ -74,6 +74,32 @@ like the username and extensions that may be required to interface with a repository. +Command-line Flags +================== + +Mercurial's default command-line parser is designed for humans, and is not +robust against malicious input. For instance, you can start a debugger by +passing ``--debugger`` as an option value:: + + $ REV=--debugger sh -c 'hg log -r "$REV"' + +This happens because several command-line flags need to be scanned without +using a concrete command table, which may be modified while loading repository +settings and extensions. + +Since Mercurial 4.4.2, the parsing of such flags may be restricted by setting +``HGPLAIN=+strictflags``. When this feature is enabled, all early options +(e.g. ``-R/--repository``, ``--cwd``, ``--config``) must be specified first +amongst the other global options, and cannot be injected to an arbitrary +location:: + + $ HGPLAIN=+strictflags hg -R "$REPO" log -r "$REV" + +In earlier Mercurial versions where ``+strictflags`` isn't available, you +can mitigate the issue by concatenating an option value with its flag:: + + $ hg log -r"$REV" --keyword="$KEYWORD" + Consuming Command Output ========================
--- a/mercurial/ui.py Thu Nov 23 22:04:53 2017 +0900 +++ b/mercurial/ui.py Thu Nov 23 22:17:03 2017 +0900 @@ -761,6 +761,7 @@ The return value can either be - False if HGPLAIN is not set, or feature is in HGPLAINEXCEPT + - False if feature is disabled by default and not included in HGPLAIN - True otherwise ''' if ('HGPLAIN' not in encoding.environ and @@ -768,6 +769,9 @@ return False exceptions = encoding.environ.get('HGPLAINEXCEPT', '').strip().split(',') + # TODO: add support for HGPLAIN=+feature,-feature syntax + if '+strictflags' not in encoding.environ.get('HGPLAIN', '').split(','): + exceptions.append('strictflags') if feature and exceptions: return feature not in exceptions return True
--- a/tests/test-commandserver.t Thu Nov 23 22:04:53 2017 +0900 +++ b/tests/test-commandserver.t Thu Nov 23 22:17:03 2017 +0900 @@ -137,6 +137,20 @@ summary: 1 +check strict parsing of early options: + + >>> import os + >>> from hgclient import check, readchannel, runcommand + >>> os.environ['HGPLAIN'] = '+strictflags' + >>> @check + ... def cwd(server): + ... readchannel(server) + ... runcommand(server, ['log', '-b', '--config=alias.log=!echo pwned', + ... 'default']) + *** runcommand log -b --config=alias.log=!echo pwned default + abort: unknown revision '--config=alias.log=!echo pwned'! + [255] + check that "histedit --commands=-" can read rules from the input channel: >>> import cStringIO
--- a/tests/test-dispatch.t Thu Nov 23 22:04:53 2017 +0900 +++ b/tests/test-dispatch.t Thu Nov 23 22:17:03 2017 +0900 @@ -113,6 +113,51 @@ $ hg log -b '--config=alias.log=!echo howdy' howdy +Early options must come first if HGPLAIN=+strictflags is specified: +(BUG: chg cherry-picks early options to pass them as a server command) + +#if no-chg + $ HGPLAIN=+strictflags hg log -b --config='hooks.pre-log=false' default + abort: unknown revision '--config=hooks.pre-log=false'! + [255] + $ HGPLAIN=+strictflags hg log -b -R. default + abort: unknown revision '-R.'! + [255] + $ HGPLAIN=+strictflags hg log -b --cwd=. default + abort: unknown revision '--cwd=.'! + [255] +#endif + $ HGPLAIN=+strictflags hg log -b --debugger default + abort: unknown revision '--debugger'! + [255] + $ HGPLAIN=+strictflags hg log -b --config='alias.log=!echo pwned' default + abort: unknown revision '--config=alias.log=!echo pwned'! + [255] + + $ HGPLAIN=+strictflags hg log --config='hooks.pre-log=false' -b default + abort: option --config may not be abbreviated! + [255] + $ HGPLAIN=+strictflags hg log -q --cwd=.. -b default + abort: option --cwd may not be abbreviated! + [255] + $ HGPLAIN=+strictflags hg log -q -R . -b default + abort: option -R has to be separated from other options (e.g. not -qR) and --repository may only be abbreviated as --repo! + [255] + + $ HGPLAIN=+strictflags hg --config='hooks.pre-log=false' log -b default + abort: pre-log hook exited with status 1 + [255] + $ HGPLAIN=+strictflags hg --cwd .. -q -Ra log -b default + 0:cb9a9f314b8b + +For compatibility reasons, HGPLAIN=+strictflags is not enabled by plain HGPLAIN: + + $ HGPLAIN= hg log --config='hooks.pre-log=false' -b default + abort: pre-log hook exited with status 1 + [255] + $ HGPLAINEXCEPT= hg log --cwd .. -q -Ra -b default + 0:cb9a9f314b8b + [defaults] $ hg cat a