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.
--- a/mercurial/subrepo.py Tue Jan 08 21:51:54 2019 +0900
+++ b/mercurial/subrepo.py Tue Jan 08 22:07:45 2019 +0900
@@ -403,7 +403,16 @@
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():
--- a/tests/test-audit-subrepo.t Tue Jan 08 21:51:54 2019 +0900
+++ b/tests/test-audit-subrepo.t Tue Jan 08 22:07:45 2019 +0900
@@ -151,20 +151,37 @@
-----------------
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 @@
properly. Any local repository paths are expanded.
on commit:
-BROKEN: wrong error message
$ mkdir envvar
$ cd envvar
@@ -230,7 +246,7 @@
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 @@
$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 @@
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 @@
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 @@
main7
main8
main9
- out-of-tree-write
$ cd ..
Test tilde
@@ -463,7 +486,6 @@
$ FAKEHOME="$TESTTMP/envvarsym/fakehome"
on commit:
-BROKEN: wrong error message
$ mkdir envvarsym
$ cd envvarsym
@@ -479,7 +501,7 @@
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 @@
$ 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 @@
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 @@
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 @@
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