1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
|
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1546953576 -32400
# Node ID 83377b4b4ae0e9a6b8e579f7b0a693b8cf5c3b10
# Parent 6c10eba6b9cddab020de49fd4fabcb2cadcd85d0
subrepo: reject potentially unsafe subrepo paths (BC) (SEC)
In addition to the previous patch, this prohibits '~', '$nonexistent', etc.
for any subrepo types. I think this is safer, and real-world subrepos wouldn't
use such (local) paths.
diff -r 6c10eba6b9cd -r 83377b4b4ae0 mercurial/subrepo.py
--- a/mercurial/subrepo.py Tue Jan 08 22:07:45 2019 +0900
+++ b/mercurial/subrepo.py Tue Jan 08 22:19:36 2019 +0900
@@ -115,6 +115,10 @@
vfs.unlink(vfs.reljoin(dirname, f))
def _auditsubrepopath(repo, path):
+ # sanity check for potentially unsafe paths such as '~' and '$FOO'
+ if path.startswith('~') or '$' in path or util.expandpath(path) != path:
+ raise error.Abort(_('subrepo path contains illegal component: %s')
+ % path)
# auditor doesn't check if the path itself is a symlink
pathutil.pathauditor(repo.root)(path)
if repo.wvfs.islink(path):
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1546952865 -32400
# 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 -r 31286c9282df -r 6c10eba6b9cd mercurial/subrepo.py
--- 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():
|