Tell `git revert` not to follow renames

Otherwise, we have an authorization bypass vulnerability: rcs_preprevert
looks at what changed in the commit we are reverting, not at what would
result from reverting it now. In particular, if some files were renamed
since the commit we are reverting, a revert of changes that were within
the designated subdirectory and allowed by check_canchange() might now
affect files that are outside the designated subdirectory or disallowed
by check_canchange().

Signed-off-by: Simon McVittie <smcv@debian.org>

diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
index 249338d4d152763d3574d19e123cd23a39e44666..7511f09cb3e94241f3f1fe6b0f2de0d2935de073 100644 (file)
--- a/IkiWiki/Plugin/git.pm
+++ b/IkiWiki/Plugin/git.pm
@@ -973,7 +973,9 @@ sub rcs_revert ($) {
 
        ensure_committer();
 
-       if (run_or_non('git', 'revert', '--no-commit', $sha1)) {
+       if (run_or_non('git', 'revert', '--strategy=recursive',
+                       '--strategy-option=no-renames',
+                       '--no-commit', $sha1)) {
                return undef;
        }
        else {

diff --git a/doc/bugs/rcs_revert_can_bypass_authorization_if_affected_files_were_renamed.mdwn b/doc/bugs/rcs_revert_can_bypass_authorization_if_affected_files_were_renamed.mdwn
index 09a23793d7555ec0c653afe430924faabb787a39..f8e3b59a3afa8e39fef3ae5924aa6ab9f9a29a3d 100644 (file)
--- a/doc/bugs/rcs_revert_can_bypass_authorization_if_affected_files_were_renamed.mdwn
+++ b/doc/bugs/rcs_revert_can_bypass_authorization_if_affected_files_were_renamed.mdwn
@@ -16,3 +16,10 @@ when reverting.
 > vulnerabilities (such as authorization bypass) by private email to the
 > maintainers, so that they are not visible to the general public
 > until we have had a chance to fix the bug. --[[smcv]]
+
+> Fixed by using
+> `git revert --strategy=recursive --strategy-option=no-renames`.
+> I tried to do something more clever (doing the revert, and checking
+> whether it made changes that aren't allowed) but couldn't get it to
+> work in a reasonable time, so I'm going with the simpler fix.
+> [[Fix committed|done]], a release will follow later today. --[[smcv]]