Merged r17060 from trunk to 3.2-stable (#27516)

mercurial: reject malicious command argument

We've got a security report from the Phabricator team, which basically says
--config and --debugger arguments can be injected anywhere to lead to an
arbitrary command execution.

https://secure.phabricator.com/rPa7921a4448093d00defa8bd18f35b8c8f8bf3314

This is a fundamental issue of the argument parsing rules in Mercurial, which
allows extensions to populate their parsing rules and such extensions can be
loaded by "--config extensions.<name>=". There's a chicken and egg problem.
We're working on hardening the parsing rules, but which won't come in by
default as it would be a behavior change.

This patch adds a verification to reject malicious command arguments as a
last ditch. The subsequent patches will fix the problem in more appropriate
way.

Contributed by Yuya Nishihara.

git-svn-id: http://svn.redmine.org/redmine/branches/3.2-stable@17074 e93f8b46-1217-0410-a6f0-8f06a7374b81

1 files changed, 15 insertions, 0 deletions

diff --git a/lib/redmine/scm/adapters/mercurial_adapter.rb b/lib/redmine/scm/adapters/mercurial_adapter.rb
index ccd9ee9c6..72f6ddfa8 100644
--- a/lib/redmine/scm/adapters/mercurial_adapter.rb
+++ b/lib/redmine/scm/adapters/mercurial_adapter.rb
@@ -32,6 +32,8 @@ module Redmine
 
         # raised if hg command exited with error, e.g. unknown revision.
         class HgCommandAborted < CommandFailed; end
+        # raised if bad command argument detected before executing hg.
+        class HgCommandArgumentError < CommandFailed; end
 
         class << self
           def client_command
@@ -286,8 +288,21 @@ module Redmine
           end
         end
 
+        # command options which may be processed earlier, by faulty parser in hg
+        HG_EARLY_BOOL_ARG = /^--(debugger|profile|traceback)$/
+        HG_EARLY_LIST_ARG = /^(--(config|cwd|repo(sitory)?)\b|-R)/
+        private_constant :HG_EARLY_BOOL_ARG, :HG_EARLY_LIST_ARG
+
         # Runs 'hg' command with the given args
         def hg(*args, &block)
+          # as of hg 4.4.1, early parsing of bool options is not terminated at '--'
+          if args.any? { |s| s =~ HG_EARLY_BOOL_ARG }
+            raise HgCommandArgumentError, "malicious command argument detected"
+          end
+          if args.take_while { |s| s != '--' }.any? { |s| s =~ HG_EARLY_LIST_ARG }
+            raise HgCommandArgumentError, "malicious command argument detected"
+          end
+
           repo_path = root_url || url
           full_args = ['-R', repo_path, '--encoding', 'utf-8']
           full_args << '--config' << "extensions.redminehelper=#{HG_HELPER_EXT}"