eval() is evil! Improper escaping allowed, at a minimum, arbitrary global

variable interpolations into Special:Upload output on malicious data
in MediaWiki:Licenses.

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 4e70c21..6224efc 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -74,6 +74,7 @@ fully support the editing toolbar, but was found to be too confusing.
 * Support for a license selection box on Special:Upload, configurable from MediaWiki:Licenses
 * Security fix for <math>
 * Security fix for tables
+* Security fix for Special:Upload license selection list
 
 
 === Caveats ===

diff --git a/includes/Licenses.php b/includes/Licenses.php
index 1d78c31..316aacd 100644 (file)
--- a/includes/Licenses.php
+++ b/includes/Licenses.php
@@ -60,8 +60,7 @@ class Licenses {
                                
                                if ( strpos( $line, '|' ) !== false ) {
                                        $obj = new License( $line );
-                                       // TODO: Do this without using eval()
-                                       eval( '$this->licenses' . $this->makeIndexes( $levels ) . '[] = $obj;' );
+                                       $this->stackItem( $this->licenses, $levels, $obj );
                                } else {
                                        if ( $level < count( $levels ) )
                                                $levels = array_slice( $levels, count( $levels ) - $level );
@@ -84,15 +83,14 @@ class Licenses {
                return array( $count, ltrim( $str, '* ' ) );
        }
        
-       function makeIndexes( $arr ) {
-               $str = '';
-       
-               wfSuppressWarnings();
-               foreach ( $arr as $item )
-                       $str .= '["' . addslashes( $item ) . '"]';
-               
-               wfRestoreWarnings();
-               return $str;
+       function stackItem( &$list, $path, $item ) {
+               $position =& $list;
+               if( $path ) {
+                       foreach( $path as $key ) {
+                               $position =& $position[$key];
+                       }
+               }
+               $position[] = $item;
        }
 
        function makeHtml( &$tagset, $depth = 0 ) {