# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 2019-01-08 13:07:45
# Node ID 6c10eba6b9cddab020de49fd4fabcb2cadcd85d0
# Parent  31286c9282dfa734e9da085649b7ae5a8ba290ad

subrepo: prohibit variable expansion on creation of hg subrepo (SEC)

It's probably wrong to expand path at localrepo.*repository() layer, but
fixing the layering issue would require careful inspection of call paths.
So, this patch adds add a validation to the subrepo constructor.

os.path.realpath(util.expandpath(root)) is what vfsmod.vfs() would do.

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -403,7 +403,16 @@ class hgsubrepo(abstractsubrepo):
         r = ctx.repo()
         root = r.wjoin(path)
         create = allowcreate and not r.wvfs.exists('%s/.hg' % path)
+        # repository constructor does expand variables in path, which is
+        # unsafe since subrepo path might come from untrusted source.
+        if os.path.realpath(util.expandpath(root)) != root:
+            raise error.Abort(_('subrepo path contains illegal component: %s')
+                              % path)
         self._repo = hg.repository(r.baseui, root, create=create)
+        if self._repo.root != root:
+            raise error.ProgrammingError('failed to reject unsafe subrepo '
+                                         'path: %s (expanded to %s)'
+                                         % (root, self._repo.root))
 
         # Propagate the parent's --hidden option
         if r is r.unfiltered():
diff --git a/tests/test-audit-subrepo.t b/tests/test-audit-subrepo.t
--- a/tests/test-audit-subrepo.t
+++ b/tests/test-audit-subrepo.t
@@ -151,20 +151,37 @@ Test current path
 -----------------
 
 on commit:
-BROKEN: should fail
 
   $ hg init currentpath
   $ cd currentpath
   $ hg init sub
   $ echo '. = sub' >> .hgsub
   $ hg ci -qAm 'add subrepo "."'
+  abort: subrepo path contains illegal component: .
+  [255]
+
+prepare tampered repo (including the commit above):
+
+  $ hg import --bypass -qm 'add subrepo "."' - <<'EOF'
+  > diff --git a/.hgsub b/.hgsub
+  > new file mode 100644
+  > --- /dev/null
+  > +++ b/.hgsub
+  > @@ -0,0 +1,1 @@
+  > +.= sub
+  > diff --git a/.hgsubstate b/.hgsubstate
+  > new file mode 100644
+  > --- /dev/null
+  > +++ b/.hgsubstate
+  > @@ -0,0 +1,1 @@
+  > +0000000000000000000000000000000000000000 .
+  > EOF
   $ cd ..
 
 on clone (and update):
 
-  $ hg clone -q currentpath currentpath2 --config ui.timeout=1
-  waiting for lock on working directory of $TESTTMP/currentpath2/. * (glob)
-  abort: working directory of $TESTTMP/currentpath2/.: timed out waiting for lock held by '*' (glob)
+  $ hg clone -q currentpath currentpath2
+  abort: subrepo path contains illegal component: .
   [255]
 
 Test outer path
@@ -214,7 +231,6 @@ Subrepository paths shouldn't be expande
 properly. Any local repository paths are expanded.
 
 on commit:
-BROKEN: wrong error message
 
   $ mkdir envvar
   $ cd envvar
@@ -230,7 +246,7 @@ BROKEN: wrong error message
   39eb4b4d3e096527668784893a9280578a8f38b8
   $ echo '$SUB = sub1' >> .hgsub
   $ SUB=sub1 hg ci -qAm 'add subrepo "$SUB"'
-  abort: repository $TESTTMP/envvar/main/$SUB already exists!
+  abort: subrepo path contains illegal component: $SUB
   [255]
 
 prepare tampered repo (including the changes above as two commits):
@@ -267,20 +283,23 @@ on clone (and update) with various subst
   $SUB
 
   $ SUB=sub1 hg clone -q main main3
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls main3
-  sub1
 
   $ SUB=sub2 hg clone -q main main4
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls main4
-  sub2
 
 on clone empty subrepo into .hg, then pull (and update), which at least fails:
-BROKEN: the first clone should fail
 
   $ SUB=.hg hg clone -qr0 main main5
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls main5
-  $ ls -d main5/.hg/.hg
-  main5/.hg/.hg
+  $ test -d main5/.hg/.hg
+  [1]
   $ SUB=.hg hg -R main5 pull -u
   pulling from $TESTTMP/envvar/main
   searching for changes
@@ -289,7 +308,8 @@ BROKEN: the first clone should fail
   adding file changes
   added 1 changesets with 1 changes to 1 files
   new changesets 7a2f0e59146f
-  abort: repository $TESTTMP/envvar/main5/$SUB already exists!
+  .hgsubstate: untracked file differs
+  abort: untracked files in working directory differ from files in requested revision
   [255]
   $ cat main5/.hg/hgrc | grep pwned
   [1]
@@ -297,32 +317,36 @@ BROKEN: the first clone should fail
 on clone (and update) into .hg, which at least fails:
 
   $ SUB=.hg hg clone -q main main6
-  abort: destination '$TESTTMP/envvar/main6/.hg' is not empty (in subrepository ".hg")
+  abort: subrepo path contains illegal component: $SUB
   [255]
   $ ls main6
   $ cat main6/.hg/hgrc | grep pwned
   [1]
 
 on clone (and update) into .hg/* subdir:
-BROKEN: should fail
 
   $ SUB=.hg/foo hg clone -q main main7
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls main7
-  $ ls main7/.hg/foo
-  hgrc
+  $ test -d main7/.hg/.hg
+  [1]
 
 on clone (and update) into outer tree:
-BROKEN: should fail
 
   $ SUB=../out-of-tree-write hg clone -q main main8
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls main8
 
 on clone (and update) into e.g. $HOME, which doesn't work since subrepo paths
 are concatenated prior to variable expansion:
 
   $ SUB="$TESTTMP/envvar/fakehome" hg clone -q main main9
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls main9 | wc -l
-  \s*1 (re)
+  \s*0 (re)
 
   $ ls
   main
@@ -334,7 +358,6 @@ are concatenated prior to variable expan
   main7
   main8
   main9
-  out-of-tree-write
   $ cd ..
 
 Test tilde
@@ -463,7 +486,6 @@ Test symlink traversal by variable expan
   $ FAKEHOME="$TESTTMP/envvarsym/fakehome"
 
 on commit:
-BROKEN: wrong error message
 
   $ mkdir envvarsym
   $ cd envvarsym
@@ -479,7 +501,7 @@ BROKEN: wrong error message
   f40c9134ba1b6961e12f250868823f0092fb68a8
   $ echo '$SUB = sub1' >> .hgsub
   $ SUB="$FAKEHOME" hg ci -qAm 'add subrepo "$SUB"'
-  abort: repository $TESTTMP/envvarsym/main/$SUB already exists!
+  abort: subrepo path contains illegal component: $SUB
   [255]
 
 prepare tampered repo (including the changes above as two commits):
@@ -510,46 +532,47 @@ prepare tampered repo (including the cha
   $ cd ..
 
 on clone (and update) without fakehome directory:
-BROKEN: should fail
 
   $ rm -fR "$FAKEHOME"
   $ SUB="$FAKEHOME" hg clone -q main main2
-  $ ls "$FAKEHOME"
-  pwned
+  abort: subrepo path contains illegal component: $SUB
+  [255]
+  $ test -d "$FAKEHOME"
+  [1]
 
 on clone (and update) with empty fakehome directory:
-BROKEN: should fail
 
   $ rm -fR "$FAKEHOME"
   $ mkdir "$FAKEHOME"
   $ SUB="$FAKEHOME" hg clone -q main main3
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls "$FAKEHOME"
-  pwned
 
 on clone (and update) with non-empty fakehome directory:
-BROKEN: wrong error message
 
   $ rm -fR "$FAKEHOME"
   $ mkdir "$FAKEHOME"
   $ touch "$FAKEHOME/a"
   $ SUB="$FAKEHOME" hg clone -q main main4
-  abort: destination '$TESTTMP/envvarsym/fakehome' is not empty (in subrepository "*") (glob)
+  abort: subrepo path contains illegal component: $SUB
   [255]
   $ ls "$FAKEHOME"
   a
 
 on clone empty subrepo with non-empty fakehome directory,
 then pull (and update):
-BROKEN: the first clone should fail
 
   $ rm -fR "$FAKEHOME"
   $ mkdir "$FAKEHOME"
   $ touch "$FAKEHOME/a"
   $ SUB="$FAKEHOME" hg clone -qr1 main main5
+  abort: subrepo path contains illegal component: $SUB
+  [255]
   $ ls "$FAKEHOME"
   a
-  $ ls -d "$FAKEHOME/.hg"
-  $TESTTMP/envvarsym/fakehome/.hg
+  $ test -d "$FAKEHOME/.hg"
+  [1]
   $ SUB="$FAKEHOME" hg -R main5 pull -u
   pulling from $TESTTMP/envvarsym/main
   searching for changes
@@ -558,21 +581,23 @@ BROKEN: the first clone should fail
   adding file changes
   added 1 changesets with 1 changes to 1 files
   new changesets * (glob)
-  abort: repository $TESTTMP/envvarsym/main5/$SUB already exists!
+  .hgsubstate: untracked file differs
+  abort: untracked files in working directory differ from files in requested revision
   [255]
   $ ls "$FAKEHOME"
   a
+  $ test -d "$FAKEHOME/.hg"
+  [1]
 
 on clone empty subrepo with hg-managed fakehome directory,
 then pull (and update):
-BROKEN: wrong error message
 
   $ rm -fR "$FAKEHOME"
   $ hg init "$FAKEHOME"
   $ touch "$FAKEHOME/a"
   $ hg -R "$FAKEHOME" ci -qAm 'add fakehome file'
   $ SUB="$FAKEHOME" hg clone -qr1 main main6
-  abort: repository $TESTTMP/envvarsym/main6/$SUB already exists!
+  abort: subrepo path contains illegal component: $SUB
   [255]
   $ ls "$FAKEHOME"
   a
@@ -592,7 +617,6 @@ BROKEN: wrong error message
 
 on clone only symlink with hg-managed fakehome directory,
 then pull (and update):
-BROKEN: wrong error message
 
   $ rm -fR "$FAKEHOME"
   $ hg init "$FAKEHOME"
@@ -609,7 +633,7 @@ BROKEN: wrong error message
   adding file changes
   added 2 changesets with 3 changes to 2 files
   new changesets * (glob)
-  abort: repository $TESTTMP/envvarsym/main7/$SUB already exists!
+  abort: subrepo path contains illegal component: $SUB
   [255]
   $ ls "$FAKEHOME"
   a