Reduce false positives for PHP in MimeMagic

(bug 16583) This was detecting PHP if any of a few three-byte strings
occurred anywhere in the first 1024 bytes of the file.  This is too
paranoid -- it creates a significant number of false positives for
binary files, reportedly on the order of about one every 4096 uploads.

It's hard to see what security advantage this check every conveyed,
because it only looks in the first 1024 bytes anyway.  For the purposes
of upload it could surely be removed entirely, but I didn't check all
callers, so maybe some caller wants to guess whether the file is PHP for
some purpose other than banning it.  So for now I only removed the
checks for the shortest strings, which were most likely to get hit.

1 files changed, 10 insertions, 8 deletions

diff --git a/includes/MimeMagic.php b/includes/MimeMagic.php
index d52de9941c7a..5e28ebe59d7c 100644
--- a/includes/MimeMagic.php
+++ b/includes/MimeMagic.php
@@ -469,16 +469,18 @@ class MimeMagic {
 		}
 
 		/*
-		 * look for PHP
-		 * Check for this before HTML/XML...
-		 * Warning: this is a heuristic, and won't match a file with a lot of non-PHP before.
-		 * It will also match text files which could be PHP. :)
+		 * Look for PHP.  Check for this before HTML/XML...  Warning: this is a
+		 * heuristic, and won't match a file with a lot of non-PHP before.  It
+		 * will also match text files which could be PHP. :)
+		 *
+		 * FIXME: For this reason, the check is probably useless -- an attacker
+		 * could almost certainly just pad the file with a lot of nonsense to
+		 * circumvent the check in any case where it would be a security
+		 * problem.  On the other hand, it causes harmful false positives (bug
+		 * 16583).  The heuristic has been cut down to exclude three-character
+		 * strings like "<? ", but should it be axed completely?
 		 */
 		if( ( strpos( $head, '<?php' ) !== false ) ||
-		    ( strpos( $head, '<? ' ) !== false ) ||
-		    ( strpos( $head, "<?\n" ) !== false ) ||
-		    ( strpos( $head, "<?\t" ) !== false ) ||
-		    ( strpos( $head, "<?=" ) !== false ) ||
 
 		    ( strpos( $head, "<\x00?\x00p\x00h\x00p" ) !== false ) ||
 		    ( strpos( $head, "<\x00?\x00 " ) !== false ) ||