mercurial: protocol security updates.
- fixes Mercurial CVE for lack of permissions checking on mercurial batch commands - more strict checks for permissions, now default to push instead of pull to be always on safe side. - decypher batch commands and pick top-most permission to be used
This commit is contained in:
parent
36bd451123
commit
7d870ea83a
2 changed files with 109 additions and 7 deletions
|
|
@ -25,6 +25,7 @@ SimpleHG middleware for handling mercurial protocol request
|
|||
|
||||
import logging
|
||||
import urlparse
|
||||
import urllib
|
||||
|
||||
from rhodecode.lib import utils
|
||||
from rhodecode.lib.ext_json import json
|
||||
|
|
@ -51,24 +52,94 @@ class SimpleHg(simplevcs.SimpleVCS):
|
|||
'getbundle': 'pull',
|
||||
'stream_out': 'pull',
|
||||
'listkeys': 'pull',
|
||||
'between': 'pull',
|
||||
'branchmap': 'pull',
|
||||
'branches': 'pull',
|
||||
'clonebundles': 'pull',
|
||||
'capabilities': 'pull',
|
||||
'debugwireargs': 'pull',
|
||||
'heads': 'pull',
|
||||
'lookup': 'pull',
|
||||
'hello': 'pull',
|
||||
'known': 'pull',
|
||||
|
||||
# largefiles
|
||||
'putlfile': 'push',
|
||||
'getlfile': 'pull',
|
||||
'statlfile': 'pull',
|
||||
'lheads': 'pull',
|
||||
|
||||
'unbundle': 'push',
|
||||
'pushkey': 'push',
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def _get_xarg_headers(cls, environ):
|
||||
i = 1
|
||||
chunks = [] # gather chunks stored in multiple 'hgarg_N'
|
||||
while True:
|
||||
head = environ.get('HTTP_X_HGARG_{}'.format(i))
|
||||
if not head:
|
||||
break
|
||||
i += 1
|
||||
chunks.append(urllib.unquote_plus(head))
|
||||
full_arg = ''.join(chunks)
|
||||
pref = 'cmds='
|
||||
if full_arg.startswith(pref):
|
||||
# strip the cmds= header defining our batch commands
|
||||
full_arg = full_arg[len(pref):]
|
||||
cmds = full_arg.split(';')
|
||||
return cmds
|
||||
|
||||
@classmethod
|
||||
def _get_batch_cmd(cls, environ):
|
||||
"""
|
||||
Handle batch command send commands. Those are ';' separated commands
|
||||
sent by batch command that server needs to execute. We need to extract
|
||||
those, and map them to our ACTION_MAPPING to get all push/pull commands
|
||||
specified in the batch
|
||||
"""
|
||||
default = 'push'
|
||||
batch_cmds = []
|
||||
try:
|
||||
cmds = cls._get_xarg_headers(environ)
|
||||
for pair in cmds:
|
||||
parts = pair.split(' ', 1)
|
||||
if len(parts) != 2:
|
||||
continue
|
||||
# entry should be in a format `key ARGS`
|
||||
cmd, args = parts
|
||||
action = cls._ACTION_MAPPING.get(cmd, default)
|
||||
batch_cmds.append(action)
|
||||
except Exception:
|
||||
log.exception('Failed to extract batch commands operations')
|
||||
|
||||
# in case we failed, (e.g malformed data) assume it's PUSH sub-command
|
||||
# for safety
|
||||
return batch_cmds or [default]
|
||||
|
||||
def _get_action(self, environ):
|
||||
"""
|
||||
Maps mercurial request commands into a pull or push command.
|
||||
In case of unknown/unexpected data, it returns 'pull' to be safe.
|
||||
In case of unknown/unexpected data, it returns 'push' to be safe.
|
||||
|
||||
:param environ:
|
||||
"""
|
||||
default = 'push'
|
||||
query = urlparse.parse_qs(environ['QUERY_STRING'],
|
||||
keep_blank_values=True)
|
||||
|
||||
if 'cmd' in query:
|
||||
cmd = query['cmd'][0]
|
||||
return self._ACTION_MAPPING.get(cmd, 'pull')
|
||||
if cmd == 'batch':
|
||||
cmds = self._get_batch_cmd(environ)
|
||||
if 'push' in cmds:
|
||||
return 'push'
|
||||
else:
|
||||
return 'pull'
|
||||
return self._ACTION_MAPPING.get(cmd, default)
|
||||
|
||||
return 'pull'
|
||||
return default
|
||||
|
||||
def _create_wsgi_app(self, repo_path, repo_name, config):
|
||||
return self.scm_app.create_hg_wsgi_app(
|
||||
|
|
|
|||
|
|
@ -47,11 +47,14 @@ def get_environ(url):
|
|||
('/foo/bar?cmd=pushkey&key=tip', 'push'),
|
||||
('/foo/bar?cmd=listkeys&key=tip', 'pull'),
|
||||
('/foo/bar?cmd=changegroup&key=tip', 'pull'),
|
||||
# Edge case: unknown argument: assume pull
|
||||
('/foo/bar?cmd=unknown&key=tip', 'pull'),
|
||||
('/foo/bar?cmd=&key=tip', 'pull'),
|
||||
('/foo/bar?cmd=hello', 'pull'),
|
||||
('/foo/bar?cmd=batch', 'push'),
|
||||
('/foo/bar?cmd=putlfile', 'push'),
|
||||
# Edge case: unknown argument: assume push
|
||||
('/foo/bar?cmd=unknown&key=tip', 'push'),
|
||||
('/foo/bar?cmd=&key=tip', 'push'),
|
||||
# Edge case: not cmd argument
|
||||
('/foo/bar?key=tip', 'pull'),
|
||||
('/foo/bar?key=tip', 'push'),
|
||||
])
|
||||
def test_get_action(url, expected_action, request_stub):
|
||||
app = simplehg.SimpleHg(config={'auth_ret_code': '', 'base_path': ''},
|
||||
|
|
@ -59,6 +62,34 @@ def test_get_action(url, expected_action, request_stub):
|
|||
assert expected_action == app._get_action(get_environ(url))
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'environ, expected_xargs, expected_batch',
|
||||
[
|
||||
({},
|
||||
[''], ['push']),
|
||||
|
||||
({'HTTP_X_HGARG_1': ''},
|
||||
[''], ['push']),
|
||||
|
||||
({'HTTP_X_HGARG_1': 'cmds=listkeys+namespace%3Dphases'},
|
||||
['listkeys namespace=phases'], ['pull']),
|
||||
|
||||
({'HTTP_X_HGARG_1': 'cmds=pushkey+namespace%3Dbookmarks%2Ckey%3Dbm%2Cold%3D%2Cnew%3Dcb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b'},
|
||||
['pushkey namespace=bookmarks,key=bm,old=,new=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b'], ['push']),
|
||||
|
||||
({'HTTP_X_HGARG_1': 'namespace=phases'},
|
||||
['namespace=phases'], ['push']),
|
||||
|
||||
])
|
||||
def test_xarg_and_batch_commands(environ, expected_xargs, expected_batch):
|
||||
app = simplehg.SimpleHg
|
||||
|
||||
result = app._get_xarg_headers(environ)
|
||||
result_batch = app._get_batch_cmd(environ)
|
||||
assert expected_xargs == result
|
||||
assert expected_batch == result_batch
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'url, expected_repo_name',
|
||||
[
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue