r94462 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r94461‎ | r94462 | r94463 >
Date:14:59, 14 August 2011
Author:dantman
Status:ok (Comments)
Tags:
Comment:
This Sanitizer::EVIL_URI_PATTERN is completely inadequate for actual security as there are numerious ways to bypass blacklisting.
Since it's only used right now for paranoia in cases you currently can't actually exploit a browser we let it slide.
However this thing needs a big fat warning message next to it to avoid someone thinking this is actually a good idea for security and ending up later on using it and opening up an XSS hole in core.
Modified paths:
  • /trunk/phase3/includes/Sanitizer.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Sanitizer.php
——@@ -39,6 +39,14 @@
4040 |&\#[xX]([0-9A-Fa-f]+);
4141 |(&)/x';
4242
 43+ /**
 44+ * Blacklist for evil uris like javascript:
 45+ * WARNING: DO NOT use this in any place that actually requires blacklisting
 46+ * for security reasons. There are NUMEROUS[1] ways to bypass blacklisting, the
 47+ * only way to be secure from javascript: uri based xss vectors is to whitelist
 48+ * things that you know are safe and deny everything else.
 49+ * [1]: http://ha.ckers.org/xss.html
 50+ */
4351 const EVIL_URI_PATTERN = '!(^|\s|\*/\s*)(javascript|vbscript)([^\w]|$)!i';
4452 const XMLNS_ATTRIBUTE_PATTERN = "/^xmlns:[:A-Z_a-z-.0-9]+$/";
4553

Comments

#Comment by 😂 (talk | contribs)   19:51, 14 August 2011

Since nothing outside this class uses it, how about making it a private static rather than a const so nobody is tempted ;-)

Status & tagging log