Make audit_path more stringent.

The following properties of a path are now checked for:

    - under top-level .hg
    - starts at the root of a windows drive
    - contains ".."
    - traverses a symlink (e.g. a/symlink_here/b)
    - inside a nested repository

If any of these is true, the path is rejected.

The check for traversing a symlink is arguably stricter than necessary;
perhaps we should be checking for symlinks that point outside the
repository.

tests/test-audit-path

@@ -0,0 +1,23 b''
+#!/bin/sh
+
+hg init
+
+echo % should fail
+hg add .hg/00changelog.i
+
+mkdir a
+echo a > a/a
+hg ci -Ama
+ln -s a b
+echo b > a/b
+
+echo % should fail
+hg add b/b
+
+echo % should succeed
+hg add b
+
+echo % should still fail - maybe
+hg add b/b
+
+exit 0

tests/test-audit-path.out

@@ -0,0 +1,8 b''
+% should fail
+abort: path contains illegal component: .hg/00changelog.i
+adding a/a
+% should fail
+abort: path 'b/b' traverses symbolic link 'b'
+% should succeed
+% should still fail - maybe
+abort: path 'b/b' traverses symbolic link 'b'

mercurial/localrepo.py

@@ -69,7 +69,8 b' class localrepository(repo.repository):'
             self.encodefn = lambda x: x
             self.decodefn = lambda x: x
             self.spath = self.path
-        self.sopener = util.encodedopener(util.opener(self.spath), self.encodefn)
+        self.sopener = util.encodedopener(util.opener(self.spath),
+                                          self.encodefn)
 
         self.ui = ui.ui(parentui=parentui)
         try:

mercurial/merge.py

@@ -391,13 +391,15 b' def applyupdates(repo, action, wctx, mct'
                 repo.ui.debug(_("copying %s to %s\n") % (f, fd))
                 repo.wwrite(fd, repo.wread(f), flags)
 
+    audit_path = util.path_auditor(repo.root)
+
     for a in action:
         f, m = a[:2]
         if f and f[0] == "/":
             continue
         if m == "r": # remove
             repo.ui.note(_("removing %s\n") % f)
-            util.audit_path(f)
+            audit_path(f)
             try:
                 util.unlink(repo.wjoin(f))
             except OSError, inst:

mercurial/util.py

@@ -13,8 +13,8 b' platform-specific details from the core.'
 """
 
 from i18n import _
-import cStringIO, errno, getpass, popen2, re, shutil, sys, tempfile
-import os, threading, time, calendar, ConfigParser, locale, glob
+import cStringIO, errno, getpass, popen2, re, shutil, sys, tempfile, strutil
+import os, stat, threading, time, calendar, ConfigParser, locale, glob
 
 try:
     set = set
@@ -366,6 +366,7 b' def canonpath(root, cwd, myname):'
     if not os.path.isabs(name):
         name = os.path.join(root, cwd, name)
     name = os.path.normpath(name)
+    audit_path = path_auditor(root)
     if name != rootsep and name.startswith(rootsep):
         name = name[len(rootsep):]
         audit_path(name)
@@ -680,12 +681,45 b' def copyfiles(src, dst, hardlink=None):'
         else:
             shutil.copy(src, dst)
 
-def audit_path(path):
-    """Abort if path contains dangerous components"""
-    parts = os.path.normcase(path).split(os.sep)
-    if (os.path.splitdrive(path)[0] or parts[0] in ('.hg', '')
-        or os.pardir in parts):
-        raise Abort(_("path contains illegal component: %s") % path)
+class path_auditor(object):
+    '''ensure that a filesystem path contains no banned components.
+    the following properties of a path are checked:
+
+    - under top-level .hg
+    - starts at the root of a windows drive
+    - contains ".."
+    - traverses a symlink (e.g. a/symlink_here/b)
+    - inside a nested repository'''
+
+    def __init__(self, root):
+        self.audited = {}
+        self.root = root
+
+    def __call__(self, path):
+        if path in self.audited:
+            return
+        parts = os.path.normcase(path).split(os.sep)
+        if (os.path.splitdrive(path)[0] or parts[0] in ('.hg', '')
+            or os.pardir in parts):
+            raise Abort(_("path contains illegal component: %s") % path)
+        def check(prefix):
+            curpath = os.path.join(self.root, prefix)
+            try:
+                st = os.lstat(curpath)
+            except OSError, err:
+                if err.errno != errno.ENOENT:
+                    raise
+            else:
+                if stat.S_ISLNK(st.st_mode):
+                    raise Abort(_('path %r traverses symbolic link %r') %
+                                (path, prefix))
+                if os.path.exists(os.path.join(curpath, '.hg')):
+                    raise Abort(_('path %r is inside repo %r') %
+                                (path, prefix))
+            self.audited[prefix] = True
+        for c in strutil.rfindall(path, os.sep):
+            check(path[:c])
+        self.audited[path] = True
 
 def _makelock_file(info, pathname):
     ld = os.open(pathname, os.O_CREAT | os.O_WRONLY | os.O_EXCL)
@@ -1262,7 +1296,10 b' class opener(object):'
     """
     def __init__(self, base, audit=True):
         self.base = base
-        self.audit = audit
+        if audit:
+            self.audit_path = path_auditor(base)
+        else:
+            self.audit_path = always
 
     def __getattr__(self, name):
         if name == '_can_symlink':
@@ -1271,8 +1308,7 b' class opener(object):'
         raise AttributeError(name)
 
     def __call__(self, path, mode="r", text=False, atomictemp=False):
-        if self.audit:
-            audit_path(path)
+        self.audit_path(path)
         f = os.path.join(self.base, path)
 
         if not text and "b" not in mode:
@@ -1293,8 +1329,7 b' class opener(object):'
         return posixfile(f, mode)
 
     def symlink(self, src, dst):
-        if self.audit:
-            audit_path(dst)
+        self.audit_path(dst)
         linkname = os.path.join(self.base, dst)
         try:
             os.unlink(linkname)

tests/test-nested-repo

@@ -4,16 +4,21 b' hg init a'
 cd a
 hg init b
 echo x > b/x
+
 echo '# should print nothing'
+hg add b
 hg st
-echo '# should print ? b/x'
+
+echo '# should fail'
 hg st b/x
-
 hg add b/x
 
-echo '# should print A b/x'
+echo '# should arguably print nothing'
+hg st b
+
+echo a > a
+hg ci -Ama a
+
+echo '# should fail'
+hg mv a b
 hg st
-echo '# should forget b/x'
-hg revert --all
-echo '# should print nothing'
-hg st b

tests/test-nested-repo.out

@@ -1,8 +1,7 b''
 # should print nothing
-# should print ? b/x
-? b/x
-# should print A b/x
-A b/x
-# should forget b/x
-forgetting b/x
-# should print nothing
+# should fail
+abort: path 'b/x' is inside repo 'b'
+abort: path 'b/x' is inside repo 'b'
+# should arguably print nothing
+# should fail
+abort: path 'b/a' is inside repo 'b'