Security tweaks:

* Leave user CSS/JS off by default
* Tighten user CSS/JS preview activation
* Require logged-in edits to include an extra session credential
* ogg removed from default upload whitelist

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index b86257a..1266267 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -697,7 +697,7 @@ $wgCompressRevisions = false;
  * This is the list of preferred extensions for uploading files. Uploading files
  * with extensions not in this list will trigger a warning.
  */
-$wgFileExtensions = array( 'png', 'gif', 'jpg', 'jpeg', 'ogg' );
+$wgFileExtensions = array( 'png', 'gif', 'jpg', 'jpeg' );
 
 /** Files with these extensions will never be allowed as uploads. */
 $wgFileBlacklist = array(
@@ -912,11 +912,19 @@ $wgUseXMLparser = false ;
 $wgSkinExtensionFunctions = array();
 $wgExtensionFunctions = array();
 
-/** Allow user Javascript page? */
-$wgAllowUserJs = true;
+/**
+ * Allow user Javascript page?
+ * This enables a lot of neat customizations, but may
+ * increase security risk to users and server load.
+ */
+$wgAllowUserJs = false;
 
-/** Allow user Cascading Style Sheets (CSS)? */
-$wgAllowUserCss = true;
+/**
+ * Allow user Cascading Style Sheets (CSS)?
+ * This enables a lot of neat customizations, but may
+ * increase security risk to users and server load.
+ */
+$wgAllowUserCss = false;
 
 /** Use the site's Javascript page? */
 $wgUseSiteJs = true;

diff --git a/includes/EditPage.php b/includes/EditPage.php
index b744fa9..f645af1 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -198,11 +198,17 @@ class EditPage {
                                # If the form is incomplete, force to preview.
                                $this->preview  = true;
                        } else {
-                               # Some browsers will not report any submit button
-                               # if the user hits enter in the comment box.
-                               # The unmarked state will be assumed to be a save,
-                               # if the form seems otherwise complete.
-                               $this->preview = $request->getCheck( 'wpPreview' );
+                               if( $this->tokenOk( $request ) ) {
+                                       # Some browsers will not report any submit button
+                                       # if the user hits enter in the comment box.
+                                       # The unmarked state will be assumed to be a save,
+                                       # if the form seems otherwise complete.
+                                       $this->preview = $request->getCheck( 'wpPreview' );
+                               } else {
+                                       # Page might be a hack attempt posted from
+                                       # an external site. Preview instead of saving.
+                                       $this->preview = true;
+                               }
                        }
                        $this->save    = !$this->preview;
                        if( !preg_match( '/^\d{14}$/', $this->edittime )) {
@@ -232,6 +238,24 @@ class EditPage {
                $this->live = $request->getCheck( 'live' );
        }
 
+       /**
+        * Make sure the form isn't faking a user's credentials.
+        *
+        * @param WebRequest $request
+        * @return bool
+        * @access private
+        */
+       function tokenOk( &$request ) {
+               global $wgUser;
+               if( $wgUser->getId() == 0 ) {
+                       # Anonymous users may not have a session
+                       # open. Don't tokenize.
+                       return true;
+               } else {
+                       return $wgUser->matchEditToken( $request->getVal( 'wpEditToken' ) );
+               }
+       }
+       
        function submit() {
                $this->edit();
        }
@@ -632,6 +656,21 @@ END
 <input type='hidden' value=\"" . htmlspecialchars( $this->section ) . "\" name=\"wpSection\" />
 <input type='hidden' value=\"{$this->edittime}\" name=\"wpEdittime\" />\n" );
 
+               if ( 0 != $wgUser->getID() ) {
+                       /**
+                        * To make it harder for someone to slip a user a page
+                        * which submits an edit form to the wiki without their
+                        * knowledge, a random token is associated with the login
+                        * session. If it's not passed back with the submission,
+                        * we won't save the page, or render user JavaScript and
+                        * CSS previews.
+                        */
+                       $token = htmlspecialchars( $wgUser->editToken() );
+                       $wgOut->addHTML( "
+<input type='hidden' value=\"$token\" name=\"wpEditToken\" />\n" );
+               }
+               
+               
                if ( $isConflict ) {
                        require_once( "DifferenceEngine.php" );
                        $wgOut->addHTML( "<h2>" . wfMsg( "yourdiff" ) . "</h2>\n" );

diff --git a/includes/Skin.php b/includes/Skin.php
index 33b4a7e..b6b7a4d 100644 (file)
--- a/includes/Skin.php
+++ b/includes/Skin.php
@@ -194,6 +194,30 @@ class Skin extends Linker {
                return $r;
        }
 
+       /**
+        * To make it harder for someone to slip a user a fake
+        * user-JavaScript or user-CSS preview, a random token
+        * is associated with the login session. If it's not
+        * passed back with the preview request, we won't render
+        * the code.
+        *
+        * @param string $action
+        * @return bool
+        * @access private
+        */
+       function userCanPreview( $action ) {
+               global $wgTitle, $wgRequest, $wgUser;
+               
+               if( $action != 'submit' )
+                       return false;
+               if( !$wgRequest->wasPosted() )
+                       return false;
+               if( !$wgTitle->userCanEditCssJsSubpage() ) 
+                       return false;
+               return $wgUser->matchEditToken(
+                       $wgRequest->getVal( 'wpEditToken' ) );
+       }
+       
        # get the user/site-specific stylesheet, SkinPHPTal called from RawPage.php (settings are cached that way)
        function getUserStylesheet() {
                global $wgOut, $wgStylePath, $wgContLang, $wgUser, $wgRequest, $wgTitle, $wgAllowUserCss;
@@ -202,7 +226,7 @@ class Skin extends Linker {
                $s = "@import \"$wgStylePath/$sheet\";\n";
                if($wgContLang->isRTL()) $s .= "@import \"$wgStylePath/common/common_rtl.css\";\n";
                if( $wgAllowUserCss && $wgUser->getID() != 0 ) { # logged in
-                       if($wgTitle->isCssSubpage() and $action == 'submit' and  $wgTitle->userCanEditCssJsSubpage()) {
+                       if($wgTitle->isCssSubpage() && $this->userCanPreview( $action ) ) {
                                $s .= $wgRequest->getText('wpTextbox1');
                        } else {
                                $userpage = $wgContLang->getNsText( Namespace::getUser() ) . ":" . $wgUser->getName();

diff --git a/includes/SkinTemplate.php b/includes/SkinTemplate.php
index 3289c5f..355ce6d 100644 (file)
--- a/includes/SkinTemplate.php
+++ b/includes/SkinTemplate.php
@@ -825,7 +825,7 @@ class SkinTemplate extends Skin {
                        $action = $wgRequest->getText('action');
                        
                        # if we're previewing the CSS page, use it
-                       if($wgTitle->isCssSubpage() and $action == 'submit' and  $wgTitle->userCanEditCssJsSubpage()) {
+                       if( $wgTitle->isCssSubpage() and $this->userCanPreview( $action ) ) {
                                $siteargs = "&smaxage=0&maxage=0";
                                $usercss = $wgRequest->getText('wpTextbox1');
                        } else {
@@ -864,7 +864,7 @@ class SkinTemplate extends Skin {
                $action = $wgRequest->getText('action');
 
                if( $wgAllowUserJs && $this->loggedin ) {
-                       if($wgTitle->isJsSubpage() and $action == 'submit' and  $wgTitle->userCanEditCssJsSubpage()) {
+                       if( $wgTitle->isJsSubpage() and $this->userCanPreview( $action ) ) {
                                # XXX: additional security check/prompt?
                                $this->userjsprev = '/*<![CDATA[*/ ' . $wgRequest->getText('wpTextbox1') . ' /*]]>*/';
                        } else {

diff --git a/includes/User.php b/includes/User.php
index ec31dc4..a116389 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -1187,6 +1187,39 @@ class User {
                }
                return false;
        }
+       
+       /**
+        * Initialize (if necessary) and return a session token value
+        * which can be used in edit forms to show that the user's
+        * login credentials aren't being hijacked with a foreign form
+        * submission.
+        *
+        * @return string
+        * @access public
+        */
+       function editToken() {
+               if( !isset( $_SESSION['wsEditToken'] ) ) {
+                       $token = dechex( mt_rand() ) . dechex( mt_rand() );
+                       $_SESSION['wsEditToken'] = $token;
+               }
+               return $_SESSION['wsEditToken'];
+       }
+       
+       /**
+        * Check given value against the token value stored in the session.
+        * A match should confirm that the form was submitted from the
+        * user's own login session, not a form submission from a third-party
+        * site.
+        *
+        * @param string $val
+        * @return bool
+        * @access public
+        */
+       function matchEditToken( $val ) {
+               if( !isset( $_SESSION['wsEditToken'] ) ) 
+                       return false;
+               return $_SESSION['wsEditToken'] == $val;
+       }
 }
 
 ?>