1353471 - Potential issue with HG pash accepting --debugger option from remote SSH sessions allowing user to drop to Pdb prompt

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

During the course of testing our HG instance I found a potential lead on getting HG to observe the --debugger command.  In my testing locally, I recall being able to get the HG drop me to a pdb prompt.  I was unable to get this working remotely, but in talking with gps recently, this deserves a bug with some follow-up and potential validation.

Here's the commands I was using to generate this condition:

ssh <email>@<server> hg -R <repo_name> serve --stdio
ssh [e-mail]@hg.mozilla.org hg -R --debugger serve --stdio

When I run this locally, I can get a Pdb shell, like so, which gives me full Python code exec capability:

$ hg -R --debugger serve --stdio
entering debugger - type c to continue starting hg or h for help
--Call--
> /usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py(21)__exit__()
-> def __exit__(self, type, value, traceback):
(Pdb) 

I just didn't have enough visibility to know if this was successful executing server-side.

A potential mitigation to this would be to invalidate any repos supplied by an end-user that contained a --debugger switch equivalent.

Comment 1

[Gregory Szorc [:gps]]

Good find. The fact you were able to get a pdb shell locally via `hg serve --debugger --stdio` is *very* concerning. I suspect this is exploitable via SSH, although possibly just not with hg.mozilla.org because of our "pash" wrapper.

I've CCd Augie Fackler, who is a Mercurial core developer and can be trusted with security-sensitive matters.

Comment 2

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

:gps - many thanks, if there is anything I can do to help/assist/patch/etc., please let me know.

Comment 3

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

I believe the --debugger in the case of hg.mo is being passed through due to a deficient matching strategy on repos.

"--debugger ".match(/^(\w|\-|\/|\.\w)+\s*$/)
 => #<MatchData "--debugger " 1:"r"> 

The affected regex lives here:

https://reviewboard-hg.mozilla.org/version-control-tools/file/tip/hgserver/pash/hg_helper.py#l129

This is why it I chose to dig in more.  I'm not exactly sure what in our instance is preventing this or if it's actually being executed and I just don't have the ability to interact with it.

Comment 4

[Gregory Szorc [:gps]]

I'm also CCing Kevin Bullock, who is another Mercurial core contributor who can be trusted.

Comment 5

[Gregory Szorc [:gps]]

I'm convinced there is a class of vulnerabilities around debugger usage.

Mercurial's core command dispatch logic in dispatch.py looks for --debugger and `--config ui.debugger=` to enable the debugger. This runs *before* `hg serve` is processed. I don't see any obvious code to prevent the debugger from running when an SSH process invokes `hg serve`.

This is arguably a feature not a bug: it is on the server operator to prevent the use of debugger. But I don't think this is documented anywhere. We definitely want a mechanism on servers to prevent the debugger from being used by client-initiated requests.

Comment 6

[Gregory Szorc [:gps]]

The hg-ssh Python script that hg ships doesn't appear impacted by this due to the way it argument processing:

    if cmdargv[:2] == ['hg', '-R'] and cmdargv[3:] == ['serve', '--stdio']:
        path = cmdargv[2]

Comment 7

[Kevin Bullock]

I'm not sure that this is unexpected, given hg serve's security model: if you have SSH access that isn't restricted to a particular command (hg serve --stdio -R /path/to/repository), then Mercurial can't prevent you from doing whatever your user has rights on the machine to do.

I wonder, though, if this is somehow exploitable via a user's .hgrc, and/or a custom extension that starts the debugger (or a Python REPL, like Mercurial's bundled config/debugshell.py) automatically? Is there anything hg can do in that case?

Comment 8

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

:gps - can you provide a path/link to that code?  I'm not convinced based on the snippet in comment 6 that this is prevented.

Comment 9

[Augie Fackler]

IMO this is a security bug of sorts: mercurial-server et al have always assumed that if you restrict to hg serve --stdio, you've contained the user to only hg actions.

I think the right thing is to add a new config knob that adds a [ui] allowdebugger = <bool>, and default it to false. Users could then re-enable the debugger, so a first-run --debugger experience would be something like:

  $ hg --debugger whatever
  abort: --debugger can open security problems if enabled on servers. Allow its use with ui.allowdebugger in your hgrc.

or similar. I'm inclined to do a new 4.1.x release to handle this.

Comment 10

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

:gps - here's a little more context from hg-ssh which seems prevent "--debugger" from being allowed, it's restricted by a path whitelist. I agree with your assessment from comment 6 now.

 allowed_paths = [os.path.normpath(os.path.join(cwd,
                                                   os.path.expanduser(path)))

--SNIP--

    if cmdargv[:2] == ['hg', '-R'] and cmdargv[3:] == ['serve', '--stdio']:
        path = cmdargv[2]
        repo = os.path.normpath(os.path.join(cwd, os.path.expanduser(path)))
        if repo in allowed_paths:

Comment 11

[Gregory Szorc [:gps]]

I agree with claudijd that Mozilla's pash code for parsing the command string feels... inadequate. I'll grab lunch then rewrite it this afternoon. Will do that in a separate bug. Let's keep this bug for the upstream Mercurial issue.

I'll be AFK for 30-45 minutes...

Comment 12

[Kevin Bullock]

I'm not quite sure yet what to do about this, but here are some observations:

It looks like the specific problem here is that `--debugger` gets processed early, even before -R(?). I worry that adding a config knob doesn't go far enough: a user that can write an hgrc file could just re-enable the debugger on the server.

The invocation `hg -R --debugger ...`, no matter what the `...` is, is erroneous and should under no circumstances do anything besides abort with an error.

If `hg serve` is invoked via a command specified in the SSH authorized_keys file (e.g. if you're using contrib/hg-ssh), there shouldn't be much of an opportunity for the user to invoke a debugger via the cmdline. But the repo path still has to be taken from the original command, so it's possible that a constructed invocation like `hg -R --debugger ...` would get thru hg-ssh.

Comment 13

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

> If `hg serve` is invoked via a command specified in the SSH authorized_keys
> file (e.g. if you're using contrib/hg-ssh), there shouldn't be much of an
> opportunity for the user to invoke a debugger via the cmdline. But the repo
> path still has to be taken from the original command, so it's possible that
> a constructed invocation like `hg -R --debugger ...` would get thru hg-ssh.

Kevin: I believe the code snippet in comment 10 prevents this because is requires the repo argument ("--debugger" in our test case) to be a valid path.  So I think to actually exploit it, you'd need to be a context where the repo path is is also "--debugger" for this to work via contrib/hg-ssh.  I think this unlikely, because the path seems to be normalized and likely is an absolute path.

Comment 14

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

> It looks like the specific problem here is that `--debugger` gets processed
> early, even before -R(?). I worry that adding a config knob doesn't go far
> enough: a user that can write an hgrc file could just re-enable the debugger
> on the server.

I was going to argue against this being possible, but thinking about it more, I agree, you could simply supply a local hgrc and it would likely be observed.  It would require two steps (1) push the hgrc and (2) leverage the --debugger call, but that all seems reasonable and within the users capability.

Comment 15

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

(In reply to Jonathan Claudius [:claudijd] (use NEEDINFO) from comment #14)
> > It looks like the specific problem here is that `--debugger` gets processed
> > early, even before -R(?). I worry that adding a config knob doesn't go far
> > enough: a user that can write an hgrc file could just re-enable the debugger
> > on the server.
> 
> I was going to argue against this being possible, but thinking about it
> more, I agree, you could simply supply a local hgrc and it would likely be
> observed.  It would require two steps (1) push the hgrc and (2) leverage the
> --debugger call, but that all seems reasonable and within the users
> capability.

Also, thinking about this a bit more.  In order to supply a localized hgrc, you'd need to be in a repo context, which you're not currently.  So, I think, you'd be bound by the current users -OR- hg process's ~/.hgrc.  So, depending on the implementation, that could be controllable by the remote user.

Comment 16

[Gregory Szorc [:gps]]

(In reply to Augie Fackler from comment #9)
> IMO this is a security bug of sorts: mercurial-server et al have always
> assumed that if you restrict to hg serve --stdio, you've contained the user
> to only hg actions.

This point is important to not overlook. The server *must* execute `hg serve --stdio` at some point. So the problem becomes how do you minimize the surface area for exploiting that process invocation. Every extra argument to `hg` is a vector for attack. We know that `-R <path>` must also be passed. But everything else is optional.

> I think the right thing is to add a new config knob that adds a [ui]
> allowdebugger = <bool>, and default it to false. Users could then re-enable
> the debugger, so a first-run --debugger experience would be something like:
> 
>   $ hg --debugger whatever
>   abort: --debugger can open security problems if enabled on servers. Allow
> its use with ui.allowdebugger in your hgrc.


I support this. But I challenge whether it is far enough.

Following the principal of reducing attack surface area, I think what I'd like to see is the default behavior to reject *all* arguments to `hg serve --stdio` except for -R. I know that could be a bit difficult to implement. But not doing this leaves the door open to --config and other arguments sneaking in there and doing who knows what. A server operator is only one mistake away from allowing extra arguments which could lead to unexpected behavior or RCE.

Of course, you can counter by saying there is still a large attack vector in whatever code is parsing `hg -R <path> serve --stdio` on the server. But I still like Mercurial providing defense in depth here.

Comment 17

[Augie Fackler]

(In reply to Jonathan Claudius [:claudijd] (use NEEDINFO) from comment #15)
> (In reply to Jonathan Claudius [:claudijd] (use NEEDINFO) from comment #14)
> > > It looks like the specific problem here is that `--debugger` gets processed
> > > early, even before -R(?). I worry that adding a config knob doesn't go far
> > > enough: a user that can write an hgrc file could just re-enable the debugger
> > > on the server.
> > 
> > I was going to argue against this being possible, but thinking about it
> > more, I agree, you could simply supply a local hgrc and it would likely be
> > observed.  It would require two steps (1) push the hgrc and (2) leverage the
> > --debugger call, but that all seems reasonable and within the users
> > capability.
> 
> Also, thinking about this a bit more.  In order to supply a localized hgrc,
> you'd need to be in a repo context, which you're not currently.  So, I
> think, you'd be bound by the current users -OR- hg process's ~/.hgrc.  So,
> depending on the implementation, that could be controllable by the remote
> user.

If the user can write files to arbitrary filesystem locations, they could also install hooks that can execute arbitrary shell. You'd already be owned at that point.

It's true that --config on the CLI would also be a problem here. Perhaps the real "fix" is to contact mercurial-server and anyone else doing this and ensure they sanitize --config and --debugger options, or offer an environment variable that can be set to disallow behavior changes on the command line?

Comment 18

[Augie Fackler]

Rough idea that'd let us prevent this:

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -163,6 +163,13 @@ def _runcatch(req):

     def _runcatchfunc():
         try:
+            opts = {}
+            cmdargs = fancyopts.fancyopts(req.args[:], commands.globalopts,
+            if cmdargs[0] == 'serve' and '--stdio' in cmdargs:
+                assert req.args[0] == '-R'
+                assert not req.args[1].startswith('--')
+                assert req.args[2] == 'serve'
+                assert req.args[3] == '--stdio'
             debugger = 'pdb'
             debugtrace = {
                 'pdb' : pdb.set_trace

Comment 19

[Augie Fackler]

Slightly improved, catches partial command names:

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -163,6 +163,16 @@ def _runcatch(req):

     def _runcatchfunc():
         try:
+            opts = {}
+            cmdargs = fancyopts.fancyopts(req.args[:], commands.globalopts,
+            cmd = cmdargs[0]
+            aliases, entry = cmdutil.findcmd(cmd, commands.table, False)
+            realcmd = aliases[0]
+            if realcmd == 'serve' and '--stdio' in cmdargs:
+                assert req.args[0] == '-R'
+                assert not req.args[1].startswith('--')
+                assert req.args[2] == 'serve'
+                assert req.args[3] == '--stdio'
             debugger = 'pdb'
             debugtrace = {
                 'pdb' : pdb.set_trace

Comment 20

[Augie Fackler]

Still needs tests, however:

# HG changeset patch
# User Augie Fackler <[e-mail]>
# Date 1491341904 14400
#      Tue Apr 04 17:38:24 2017 -0400
# Node ID 7185b8510bac434202ce4ca421b5484f886a310a
# Parent  3629f36bc6b9c5f44e5d9b4e0a8aaa55f6063a51
dispatch: protect against malicious 'hg serve --stdio' invocations (sec)

Some shared-ssh installations assume that 'hg serve --stdio' is a safe
command to run for minimally trusted users. Unfortunately, the messy
implementation of argument parsing here meant that trying to access a
repo named '--debugger' would give the user a pdb prompt, thereby
sidestepping any hoped-for sandboxing.

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -163,6 +163,29 @@ def _runcatch(req):
 
     def _runcatchfunc():
         try:
+            opts = {}
+            cmdargs = fancyopts.fancyopts(req.args[:], commands.globalopts, opts)
+            cmd = cmdargs[0]
+            realcmd = None
+            try:
+                aliases, entry = cmdutil.findcmd(cmd, commands.table, False)
+                realcmd = aliases[0]
+            except error.UnknownCommand:
+                # Don't handle this here. We know the command is
+                # invalid, but all we're worried about for now is that
+                # it's not a command that server operators expect to
+                # be safe to offer to users in a sandbox.
+                pass
+            if realcmd == 'serve' and '--stdio' in cmdargs:
+                if (req.args[0] != '-R' or
+                    req.args[1].startswith('--') or
+                    req.args[2] != 'serve' or
+                    req.args[3] != '--stdio' or
+                    len(req.args) != 4):
+                    raise error.Abort(
+                        'potentially unsafe serve --stdio invocation: %r' %
+                        (req.args,))
+
             debugger = 'pdb'
             debugtrace = {
                 'pdb' : pdb.set_trace

Comment 21

[Augie Fackler]

Attachment: sec.diff

Final version of my patch with tests. Once this is done, I'll close the loop on mercurial-security and mercurial-security-announce.

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 8854901 [details] [diff] [review]
sec.diff

Review of attachment 8854901 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good for what it is.

I thought we were going to disable the debugger by default and require a config option to enable it. That would clamp down on this attack vector completely. As it stands, I don't think there is an explicit config option to disable the debugger. So the commit message is warning me about an attack vector which I must be a wizard to defend against. That scares me.

This patch on its own looks mostly good. Changes to disable debugger would be in a separate patch, of course.

I'll hold off granting r+ until we have the final state sorted out.

::: mercurial/dispatch.py
@@ +155,5 @@
>          pass # happens if called in a thread
>  
>      def _runcatchfunc():
> +        cmdargs = fancyopts.fancyopts(req.args[:], commands.globalopts, {})
> +        cmd = cmdargs[0]

Can there be an IndexError here?

@@ +167,5 @@
> +            # it's not a command that server operators expect to
> +            # be safe to offer to users in a sandbox.
> +            pass
> +        if realcmd == 'serve' and '--stdio' in cmdargs:
> +            if (len(req.args) != 4 or

I'd prefer to see an inline comment explaining the dragons that be here.

Comment 23

[Augie Fackler]

I'll try and get to an updated version of the patch this afternoon. I wasn't planning on whole-hog disabling --debugger in stable, or even trying to add that as a config knob, on account of it being the stable branch. Once we get the specific vector of 'hg serve --stdio' sorted, I figured we could come back to this code on default and do some refactoring to clean up. Sound good?

Comment 24

[Kevin Bullock]

Agreed, I think our goal on stable is to ensure that the `hg serve -R /path/to/repo --stdio` invocation is precisely that, i.e. that the only thing that is ever different is the repo path.

My goal on default is to make it so that nothing in dispatch needs to do things like `if '--debugger' in req.args:`, which AFAIU is the main reason this vector exists to begin with. That is, by the time we're interpreting an argument semantically, we should have already parsed and verified whether it's a flag, a flag argument, or a regular argument.

Aside from the things Greg pointed out the patch LGTM. Will look for V2.

Comment 25

[Augie Fackler]

Attachment: sec2.diff

v2 of patch, passes hg testsuite

Comment 26

[Kevin Bullock]

Comment on attachment 8856786 [details] [diff] [review]
sec2.diff

Review of attachment 8856786 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

Comment 27

[Gregory Szorc [:gps]]

Comment on attachment 8856786 [details] [diff] [review]
sec2.diff

Review of attachment 8856786 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 28

[Augie Fackler]

Thanks! I've sent out the batch of announcement emails and will reach out to bitbucket privately to make sure they got the message.

Comment 29

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

FWIW, I was able to finally get the PoC working remotely in my test environment.  It did require a patch to contrib/hg-ssh to bypass the path validation, but I think this shows that if someone's rolling their own hg-ssh wrapper, using an older one which may not have path validation, or is somehow customizing a modern contrib/hg-ssh to add functionality (like server-side cloning and the like) that this is viable means to remote RCE.

# Baseline (shows that arbitrary commands are restricted)
$ ssh root@192.168.10.99 ifconfig
Illegal command "ifconfig"

# Example remote --debugger exploitation
$ ssh root@192.168.10.99 hg -R --debugger serve --stdio
entering debugger - type c to continue starting hg or h for help
from subprocess import call;call(["whoami"])      
root

Comment 30

[Kevin Bullock]

Looks like the patch breaks hg-ssh readonly mode. With the 2nd version of the patch applied, I get the following:

--- tests/test-ssh.t	2017-04-17 19:40:53.000000000 -0500
+++ tests/test-ssh.t.err	2017-04-17 19:57:05.000000000 -0500
@@ -412,166 +412,10 @@
   > EOF
 
   $ hg clone --ssh "sh ssh.sh" "ssh://user@dummy/$TESTTMP/remote" read-only-local
-  requesting all changes
-  adding changesets
-  adding manifests
-  adding file changes
-  added 6 changesets with 5 changes to 4 files (+1 heads)
-  updating to branch default
-  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  remote: abort: potentially unsafe serve --stdio invocation: ['-R', '$TESTTMP/remote', 'serve', '--stdio', '--config', 'hooks.pretxnopen.hg-ssh=python:__main__.rejectpush', '--config', 'hooks.prepushkey.hg-ssh=python:__main__.rejectpush']
+  abort: no suitable response from remote hg!
+  [255]
 
   $ cd read-only-local
+  $TESTTMP.sh: 225: cd: can't cd to read-only-local
   $ echo "baz" > bar
-  $ hg ci -A -m "unpushable commit" bar
-  $ hg push --ssh "sh ../ssh.sh"
-  pushing to ssh://user@dummy/*/remote (glob)
-  searching for changes
-  remote: Permission denied
-  remote: pretxnopen.hg-ssh hook failed
-  abort: push failed on remote
-  [255]
-
-  $ cd ..
-
-stderr from remote commands should be printed before stdout from local code (issue4336)
-
-  $ hg clone remote stderr-ordering
-  updating to branch default
-  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  $ cd stderr-ordering
-  $ cat >> localwrite.py << EOF
-  > from mercurial import exchange, extensions
-  > 
-  > def wrappedpush(orig, repo, *args, **kwargs):
-  >     res = orig(repo, *args, **kwargs)
-  >     repo.ui.write('local stdout\n')
-  >     return res
-  > 
-  > def extsetup(ui):
-  >     extensions.wrapfunction(exchange, 'push', wrappedpush)
-  > EOF
-
-  $ cat >> .hg/hgrc << EOF
-  > [paths]
-  > default-push = ssh://user@dummy/remote
-  > [ui]
-  > ssh = python "$TESTDIR/dummyssh"
-  > [extensions]
-  > localwrite = localwrite.py
-  > EOF
-
-  $ echo localwrite > foo
-  $ hg commit -m 'testing localwrite'
-  $ hg push
-  pushing to ssh://user@dummy/remote
-  searching for changes
-  remote: adding changesets
-  remote: adding manifests
-  remote: adding file changes
-  remote: added 1 changesets with 1 changes to 1 files
-  remote: KABOOM
-  remote: KABOOM IN PROCESS
-  local stdout
-
-debug output
-
-  $ hg pull --debug ssh://user@dummy/remote
-  pulling from ssh://user@dummy/remote
-  running python ".*/dummyssh" user@dummy ('|")hg -R remote serve --stdio('|") (re)
-  sending hello command
-  sending between command
-  remote: 355
-  remote: capabilities: lookup changegroupsubset branchmap pushkey known getbundle unbundlehash batch streamreqs=generaldelta,revlogv1 bundle2=HG20%0Achangegroup%3D01%2C02%0Adigests%3Dmd5%2Csha1%2Csha512%0Aerror%3Dabort%2Cunsupportedcontent%2Cpushraced%2Cpushkey%0Ahgtagsfnodes%0Alistkeys%0Apushkey%0Aremote-changegroup%3Dhttp%2Chttps unbundle=HG10GZ,HG10BZ,HG10UN
-  remote: 1
-  query 1; heads
-  sending batch command
-  searching for changes
-  all remote heads known locally
-  no changes found
-  sending getbundle command
-  bundle2-input-bundle: with-transaction
-  bundle2-input-part: "listkeys" (params: 1 mandatory) supported
-  bundle2-input-part: total payload size 15
-  bundle2-input-part: "listkeys" (params: 1 mandatory) supported
-  bundle2-input-part: total payload size 45
-  bundle2-input-bundle: 1 parts total
-  checking for updated bookmarks
-
-  $ cd ..
-
-  $ cat dummylog
-  Got arguments 1:user@dummy 2:hg -R nonexistent serve --stdio
-  Got arguments 1:user@dummy 2:hg -R $TESTTMP/nonexistent serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R local-stream serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R doesnotexist serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R local serve --stdio
-  Got arguments 1:user@dummy 2:hg -R $TESTTMP/local serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  changegroup-in-remote hook: HG_BUNDLE2=1 HG_NODE=a28a9d1a809cab7d4e2fde4bee738a9ede948b60 HG_NODE_LAST=a28a9d1a809cab7d4e2fde4bee738a9ede948b60 HG_SOURCE=serve HG_TXNID=TXN:* HG_URL=remote:ssh:127.0.0.1 (glob)
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  changegroup-in-remote hook: HG_BUNDLE2=1 HG_NODE=1383141674ec756a6056f6a9097618482fe0f4a6 HG_NODE_LAST=1383141674ec756a6056f6a9097618482fe0f4a6 HG_SOURCE=serve HG_TXNID=TXN:* HG_URL=remote:ssh:127.0.0.1 (glob)
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  Got arguments 1:user@dummy 2:hg init 'a repo'
-  Got arguments 1:user@dummy 2:hg -R 'a repo' serve --stdio
-  Got arguments 1:user@dummy 2:hg -R 'a repo' serve --stdio
-  Got arguments 1:user@dummy 2:hg -R 'a repo' serve --stdio
-  Got arguments 1:user@dummy 2:hg -R 'a repo' serve --stdio
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-  changegroup-in-remote hook: HG_BUNDLE2=1 HG_NODE=65c38f4125f9602c8db4af56530cc221d93b8ef8 HG_NODE_LAST=65c38f4125f9602c8db4af56530cc221d93b8ef8 HG_SOURCE=serve HG_TXNID=TXN:* HG_URL=remote:ssh:127.0.0.1 (glob)
-  Got arguments 1:user@dummy 2:hg -R remote serve --stdio
-
-remote hook failure is attributed to remote
-
-  $ cat > $TESTTMP/failhook << EOF
-  > def hook(ui, repo, **kwargs):
-  >     ui.write('hook failure!\n')
-  >     ui.flush()
-  >     return 1
-  > EOF
-
-  $ echo "pretxnchangegroup.fail = python:$TESTTMP/failhook:hook" >> remote/.hg/hgrc
-
-  $ hg -q --config ui.ssh="python $TESTDIR/dummyssh" clone ssh://user@dummy/remote hookout
-  $ cd hookout
-  $ touch hookfailure
-  $ hg -q commit -A -m 'remote hook failure'
-  $ hg --config ui.ssh="python $TESTDIR/dummyssh" push
-  pushing to ssh://user@dummy/remote
-  searching for changes
-  remote: adding changesets
-  remote: adding manifests
-  remote: adding file changes
-  remote: added 1 changesets with 1 changes to 1 files
-  remote: hook failure!
-  remote: transaction abort!
-  remote: rollback completed
-  remote: pretxnchangegroup.fail hook failed
-  abort: push failed on remote
-  [255]
-
-abort during pull is properly reported as such
-
-  $ echo morefoo >> ../remote/foo
-  $ hg -R ../remote commit --message "more foo to be pulled"
-  $ cat >> ../remote/.hg/hgrc << EOF
-  > [extensions]
-  > crash = ${TESTDIR}/crashgetbundler.py
-  > EOF
-  $ hg --config ui.ssh="python $TESTDIR/dummyssh" pull
-  pulling from ssh://user@dummy/remote
-  searching for changes
-  remote: abort: this is an exercise
-  abort: pull failed on remote
-  [255]

Comment 31

[Gregory Szorc [:gps]]

Looks like this got assigned CVE-2017-9462: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-9462

A fix is in upstream Mercurial.

And, Mozilla's custom login shell was hardened in https://hg.mozilla.org/hgcustom/version-control-tools/rev/8b32413e03a8. So this bug can be resolved.

This bug can also likely be made public. But I'm rarely comfortable flipping that switch, so I defer to someone else.

Comment 32

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

I'm fine with making this public, since we're all patched up.

augie/kevin: any objections to that?  If I don't hear within a couple days, I'll assume no objections.  Thanks!

Comment 33

[Augie Fackler]

By all means let's make this public.

Comment 34

[Kevin Bullock]

Yes, let's make it public.