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.
--- a/mercurial/localrepo.py Thu Aug 09 20:16:00 2007 -0700
+++ b/mercurial/localrepo.py Fri Aug 10 10:46:03 2007 -0700
@@ -69,7 +69,8 @@
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:
--- a/mercurial/merge.py Thu Aug 09 20:16:00 2007 -0700
+++ b/mercurial/merge.py Fri Aug 10 10:46:03 2007 -0700
@@ -391,13 +391,15 @@
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:
--- a/mercurial/util.py Thu Aug 09 20:16:00 2007 -0700
+++ b/mercurial/util.py Fri Aug 10 10:46:03 2007 -0700
@@ -13,8 +13,8 @@
"""
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 @@
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 @@
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 @@
"""
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 @@
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 @@
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)
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-audit-path Fri Aug 10 10:46:03 2007 -0700
@@ -0,0 +1,23 @@
+#!/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
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-audit-path.out Fri Aug 10 10:46:03 2007 -0700
@@ -0,0 +1,8 @@
+% 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'
--- a/tests/test-nested-repo Thu Aug 09 20:16:00 2007 -0700
+++ b/tests/test-nested-repo Fri Aug 10 10:46:03 2007 -0700
@@ -4,16 +4,21 @@
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
--- a/tests/test-nested-repo.out Thu Aug 09 20:16:00 2007 -0700
+++ b/tests/test-nested-repo.out Fri Aug 10 10:46:03 2007 -0700
@@ -1,8 +1,7 @@
# 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'