From 7030f69a1ad8295facca0f4c9c5a3833cb703ecf Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 4 Feb 2005 06:19:37 +0000 Subject: [PATCH] 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 --- includes/DefaultSettings.php | 18 +++++++++---- includes/EditPage.php | 49 ++++++++++++++++++++++++++++++++---- includes/Skin.php | 26 ++++++++++++++++++- includes/SkinTemplate.php | 4 +-- includes/User.php | 33 ++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 13 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index b86257ab3c..1266267bb2 100644 --- 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 b744fa9ff0..f645af164d 100644 --- 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 section ) . "\" name=\"wpSection\" /> 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( " +\n" ); + } + + if ( $isConflict ) { require_once( "DifferenceEngine.php" ); $wgOut->addHTML( "

" . wfMsg( "yourdiff" ) . "

\n" ); diff --git a/includes/Skin.php b/includes/Skin.php index 33b4a7e790..b6b7a4ddf1 100644 --- 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 3289c5f352..355ce6d6df 100644 --- 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 = '/*getText('wpTextbox1') . ' /*]]>*/'; } else { diff --git a/includes/User.php b/includes/User.php index ec31dc4e9e..a116389938 100644 --- 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; + } } ?> -- 2.20.1