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.

mercurial/chgserver.py

@@ -220,8 +220,17 b' 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 b' 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)

mercurial/dispatch.py

@@ -150,6 +150,8 b' 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 b' 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 b' 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 b' 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:

mercurial/help/environment.txt

@@ -56,9 +56,17 b' 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:

mercurial/help/scripting.txt

@@ -74,6 +74,32 b' 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
 ========================
 

mercurial/ui.py

@@ -761,6 +761,7 b' 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 b' 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

tests/test-commandserver.t

@@ -137,6 +137,20 b' 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

tests/test-dispatch.t

@@ -113,6 +113,51 b' 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