[PATCH 2 of 2 STABLE V2] dispatch: add HGPLAIN=+strictflags to restrict early parsing of global options
Yuya Nishihara
yuya at tcha.org
Fri Nov 24 10:58:06 EST 2017
# HG changeset patch
# User Yuya Nishihara <yuya at tcha.org>
# Date 1511443023 -32400
# Thu Nov 23 22:17:03 2017 +0900
# Branch stable
# Node ID 54e6e3b35ea5a96d2d822ac23d44b4d105178c05
# Parent 78d25e7afffe6ce150da8b1ca32418d6394b0442
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.
diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -220,8 +220,17 @@ def _loadnewui(srcui, args):
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 @@ def _loadnewui(srcui, args):
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)
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -150,6 +150,8 @@ def dispatch(req):
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 @@ def _parseconfig(ui, config):
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 _earlygetopt(aliases, args, strip=Tr
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 _earlyreqoptstr(req, name, aliases):
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:
diff --git a/mercurial/help/environment.txt b/mercurial/help/environment.txt
--- a/mercurial/help/environment.txt
+++ b/mercurial/help/environment.txt
@@ -56,9 +56,17 @@ HGPLAIN
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:
diff --git a/mercurial/help/scripting.txt b/mercurial/help/scripting.txt
--- a/mercurial/help/scripting.txt
+++ b/mercurial/help/scripting.txt
@@ -74,6 +74,32 @@ HGRCPATH
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
========================
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -761,6 +761,7 @@ class ui(object):
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 @@ class ui(object):
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
diff --git a/tests/test-commandserver.t b/tests/test-commandserver.t
--- a/tests/test-commandserver.t
+++ b/tests/test-commandserver.t
@@ -137,6 +137,20 @@ typical client does not want echo-back m
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
diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
--- a/tests/test-dispatch.t
+++ b/tests/test-dispatch.t
@@ -113,6 +113,51 @@ Shell aliases bypass any command parsing
$ 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
More information about the Mercurial-devel
mailing list