[PATCH 0 of 5] keyword: refactor with 2 security enhancements

Christian Ebert blacktrash at gmx.net
Fri Oct 1 16:35:19 UTC 2010


Hi,

Refactoring the keyword extension.

2 security enhancements regarding unwanted keyword (un)expansion:

1) handle copying/renaming to a destination not configured for
   keyword expansion gracefully (2nd patch)
2) Stricter keyword detection by using 2 specific regular
   expressions which search either for unexpanded or expanded
   keywords only (3rd patch)

The first patch removes duplicate search or replace operations
on keyword (un)expansion.

With the 2nd patch of this series copy and rename operations
become safer wrt keyword expansion.

Before files containing expanded keywords were copied/renamed
unaltered to a destination ignored by the extension. If these
files were checked in the expanded keywords became
unintentionally part of the change history. I tend to consider
this as a long-standing bug in the extension, which should now be
fixed.

I don't think the wrapper for cmdutil.copy needs to be
write-locked as cmdutil.copy is "called with the repo lock held",
but I'm fine with being taught otherwise.

A weird corner case where "hg cp symlink dest" yields a different
result (symlink) as "cp symlink dest; hg cp -A symlink dest"
(regular file) is also covered and tested. However, I'm don't
know how cp behaves on other platforms and whether the test would
break there.

The 3rd patch makes keyword search and substitution more secure,
also "loosely" formatted keywords, like $Id:  $ (before
expansion) are not allowed anymore -- an issue which cropped up
at least once on the wiki.

The 4th patch moves the call to (w)ctx.flags into the iskwfile
function, which now simply takes the context as argument.

The 5th patch simplifies kwfilelog.cmp(); also avoids the import
of revlog and fiddling with it.

A crew repo with patch series applied can be found here:
http://www.blacktrash.org/hg-crew-mq/

c


More information about the Mercurial-devel mailing list