Patchwork D2717: wireprotoserver: check permissions in main dispatch function

login
register
mail settings
Submitter phabricator
Date March 9, 2018, 7:29 p.m.
Message ID <2dcdd2fba5f1b397398d27fde125324d@localhost.localdomain>
Download mbox | patch
Permalink /patch/29183/
State Not Applicable
Headers show

Comments

phabricator - March 9, 2018, 7:29 p.m.
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGc638a13093cf: wireprotoserver: check permissions in main dispatch function (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2717?vs=6711&id=6771

REVISION DETAIL
  https://phab.mercurial-scm.org/D2717

AFFECTED FILES
  mercurial/hgweb/hgweb_mod.py
  mercurial/wireprotoserver.py

CHANGE DETAILS




To: indygreg, #hg-reviewers, durin42
Cc: mercurial-devel

Patch

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -179,7 +179,8 @@ 
     return {
         'cmd': cmd,
         'proto': proto,
-        'dispatch': lambda: _callhttp(repo, req, proto, cmd),
+        'dispatch': lambda checkperm: _callhttp(repo, req, proto, cmd,
+                                                checkperm),
         'handleerror': lambda ex: _handlehttperror(ex, req, cmd),
     }
 
@@ -223,7 +224,7 @@ 
     opts = {'level': ui.configint('server', 'zliblevel')}
     return HGTYPE, util.compengines['zlib'], opts
 
-def _callhttp(repo, req, proto, cmd):
+def _callhttp(repo, req, proto, cmd, checkperm):
     def genversion2(gen, engine, engineopts):
         # application/mercurial-0.2 always sends a payload header
         # identifying the compression engine.
@@ -241,6 +242,12 @@ 
                            'over HTTP'))
         return []
 
+    # Assume commands with no defined permissions are writes /
+    # for pushes. This is the safest from a security perspective
+    # because it doesn't allow commands with undefined semantics
+    # from bypassing permissions checks.
+    checkperm(wireproto.permissions.get(cmd, 'push'))
+
     rsp = wireproto.dispatch(repo, proto, cmd)
 
     if isinstance(rsp, bytes):
diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py
+++ b/mercurial/hgweb/hgweb_mod.py
@@ -357,22 +357,15 @@ 
         protohandler = wireprotoserver.parsehttprequest(rctx.repo, req, query)
 
         if protohandler:
-            cmd = protohandler['cmd']
             try:
                 if query:
                     raise ErrorResponse(HTTP_NOT_FOUND)
 
                 # TODO fold this into parsehttprequest
-                req.checkperm = lambda op: self.check_perm(rctx, req, op)
-                protohandler['proto'].checkperm = req.checkperm
+                checkperm = lambda op: self.check_perm(rctx, req, op)
+                protohandler['proto'].checkperm = checkperm
 
-                # Assume commands with no defined permissions are writes /
-                # for pushes. This is the safest from a security perspective
-                # because it doesn't allow commands with undefined semantics
-                # from bypassing permissions checks.
-                req.checkperm(perms.get(cmd, 'push'))
-
-                return protohandler['dispatch']()
+                return protohandler['dispatch'](checkperm)
             except ErrorResponse as inst:
                 return protohandler['handleerror'](inst)