Fixes to input validation and output escaping for user preferences.

Inserting a newline into some improperly filtered option strings could be used to overwrite other pref values, bypassing their input validation. Any newlines now get filtered out at User::setOption as a final line of defence.
There were a few HTML injection bugs, but none appear to be exploitable, as prefs can only be set if you already control the account.
Bug found by gmaxwell.
This commit is contained in:
Brion Vibber 2006-05-15 09:45:14 +00:00
parent 774ce214bd
commit ce8edcc565
6 changed files with 46 additions and 26 deletions

View file

@ -271,6 +271,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN
* (bug 5952) Improvement to German localisation (de)
* Rename conflicting metadata help message to "metadata_help" (was "metadata")
and treat it as wiki text
* Improve preferences input filtering
== Compatibility ==

View file

@ -835,8 +835,8 @@ class EditPage {
$wgOut->addWikiText( wfMsg( 'longpagewarning', $wgLang->formatNum( $this->kblength ) ) );
}
$rows = $wgUser->getOption( 'rows' );
$cols = $wgUser->getOption( 'cols' );
$rows = $wgUser->getIntOption( 'rows' );
$cols = $wgUser->getIntOption( 'cols' );
$ew = $wgUser->getOption( 'editwidth' );
if ( $ew ) $ew = " style=\"width:100%\"";

View file

@ -849,8 +849,8 @@ class OutputPage {
$source = wfMsg( $wgUser->isLoggedIn() ? 'noarticletext' : 'noarticletextanon' );
}
}
$rows = $wgUser->getOption( 'rows' );
$cols = $wgUser->getOption( 'cols' );
$rows = $wgUser->getIntOption( 'rows' );
$cols = $wgUser->getIntOption( 'cols' );
$text = "\n<textarea name='wpTextbox1' id='wpTextbox1' cols='$cols' rows='$rows' readonly='readonly'>" .
htmlspecialchars( $source ) . "\n</textarea>";

View file

@ -745,7 +745,7 @@ class SkinTemplate extends Skin {
$content_actions['varlang-' . $vcount] = array(
'class' => $selected,
'text' => $varname,
'href' => $this->mTitle->getLocalUrl( $actstr . 'variant=' . $code )
'href' => $this->mTitle->getLocalUrl( $actstr . 'variant=' . urlencode( $code ) )
);
$vcount ++;
}

View file

@ -713,7 +713,7 @@ class PreferencesForm {
} else {
# Need to output a hidden option even if the relevant skin is not in use,
# otherwise the preference will get reset to 0 on submit
$wgOut->addHTML( "<input type='hidden' name='wpQuickbar' value='{$this->mQuickbar}' />" );
$wgOut->addHtml( wfHidden( 'wpQuickbar', $this->mQuickbar ) );
}
# Skin
@ -816,10 +816,11 @@ class PreferencesForm {
#
global $wgLivePreview, $wgUseRCPatrol;
$wgOut->addHTML( '<fieldset><legend>' . wfMsg( 'textboxsize' ) . '</legend>
<div>
<label for="wpRows">' . wfMsg( 'rows' ) . "</label> <input type='text' name='wpRows' id='wpRows' value=\"{$this->mRows}\" size='3' />
<label for='wpCols'>" . wfMsg( 'columns' ) . "</label> <input type='text' name='wpCols' id='wpCols' value=\"{$this->mCols}\" size='3' />
</div>" .
<div>' .
wfInputLabel( wfMsg( 'rows' ), 'wpRows', 'wpRows', 3, $this->mRows ) .
' ' .
wfInputLabel( wfMsg( 'columns' ), 'wpCols', 'wpCols', 3, $this->mCols ) .
"</div>" .
$this->getToggles( array(
'editsection',
'editsectiononrightclick',
@ -841,8 +842,8 @@ class PreferencesForm {
$this->mUsedToggles['autopatrol'] = true; # Don't show this up for users who can't; the handler below is dumb and doesn't know it
$wgOut->addHTML( '<fieldset><legend>' . htmlspecialchars(wfMsg('prefs-rc')) . '</legend>' .
'<label for="wpRecent">' . wfMsg ( 'recentchangescount' ) .
"</label> <input type='text' name='wpRecent' id='wpRecent' value=\"$this->mRecent\" size='3' />" .
wfInputLabel( wfMsg( 'recentchangescount' ),
'wpRecent', 'wpRecent', 3, $this->mRecent ) .
$this->getToggles( array(
'hideminor',
$wgRCShowWatchingUsers ? 'shownumberswatching' : false,
@ -853,38 +854,36 @@ class PreferencesForm {
# Watchlist
$wgOut->addHTML( '<fieldset><legend>' . wfMsgHtml( 'prefs-watchlist' ) . '</legend>' );
$wgOut->addHTML( '<label for="wpWatchlistDays">' . wfMsgHtml( 'prefs-watchlist-days' ) . '</label> ' );
$wgOut->addHTML( '<input type="text" name="wpWatchlistDays" id="wpWatchlistDays" value="' . $this->mWatchlistDays . '" size="3" />' );
$wgOut->addHTML( wfInputLabel( wfMsg( 'prefs-watchlist-days' ),
'wpWatchlistDays', 'wpWatchlistDays', 3, $this->mWatchlistDays ) );
$wgOut->addHTML( '<br /><br />' ); # Spacing
$wgOut->addHTML( $this->getToggles( array( 'watchlisthideown', 'watchlisthidebots', 'extendwatchlist' ) ) );
$wgOut->addHTML( '<label for="wpWatchlistEdits">' . wfMsgHtml( 'prefs-watchlist-edits' ) . '</label> ' );
$wgOut->addHTML( '<input type="text" name="wpWatchlistEdits" id="wpWatchlistEdits" value="' . $this->mWatchlistEdits . '" size="3" />' );
$wgOut->addHTML( wfInputLabel( wfMsg( 'prefs-watchlist-edits' ),
'wpWatchlistEdits', 'wpWatchlistEdits', 3, $this->mWatchlistEdits ) );
$wgOut->addHTML( '</fieldset>' );
# Search
$wgOut->addHTML( '<fieldset><legend>' . wfMsg( 'searchresultshead' ) . '</legend><table>' .
$this->addRow(
'<label for="wpSearch">' . wfMsg( 'resultsperpage' ) . '</label>',
"<input type='text' name='wpSearch' id='wpSearch' value=\"$this->mSearch\" size='4' />"
wfLabel( wfMsg( 'resultsperpage' ), 'wpSearch' ),
wfInput( 'wpSearch', 4, $this->mSearch, array( 'id' => 'wpSearch' ) )
) .
$this->addRow(
'<label for="wpSearchLines">' . wfMsg( 'contextlines' ) . '</label>',
"<input type='text' name='wpSearchLines' id='wpSearchLines' value=\"$this->mSearchLines\" size='4' />"
wfLabel( wfMsg( 'contextlines' ), 'wpSearchLines' ),
wfInput( 'wpSearchLines', 4, $this->mSearchLines, array( 'id' => 'wpSearchLines' ) )
) .
$this->addRow(
'<label for="wpSearchChars">' . wfMsg( 'contextchars' ) . '</label>',
"<input type='text' name='wpSearchChars' id='wpSearchChars' value=\"$this->mSearchChars\" size='4' />"
wfLabel( wfMsg( 'contextchars' ), 'wpSearchChars' ),
wfInput( 'wpSearchChars', 4, $this->mSearchChars, array( 'id' => 'wpSearchChars' ) )
) .
"</table><fieldset><legend>" . wfMsg( 'defaultns' ) . "</legend>$ps</fieldset></fieldset>" );
# Misc
#
$wgOut->addHTML('<fieldset><legend>' . wfMsg('prefs-misc') . '</legend>');
$wgOut->addHTML(
'<label for="wpStubs">' . htmlspecialchars ( wfMsg ( 'stubthreshold' ) ) . '</label>' .
" <input type='text' name='wpStubs' id='wpStubs' value=\"$this->mStubs\" size='6' />"
);
$wgOut->addHTML( wfInputLabel( wfMsg( 'stubthreshold' ),
'wpStubs', 'wpStubs', 6, $this->mStubs ) );
$msgUnderline = htmlspecialchars( wfMsg ( 'tog-underline' ) );
$msgUnderlinenever = htmlspecialchars( wfMsg ( 'underline-never' ) );
$msgUnderlinealways = htmlspecialchars( wfMsg ( 'underline-always' ) );

View file

@ -1069,6 +1069,20 @@ class User {
function getBoolOption( $oname ) {
return (bool)$this->getOption( $oname );
}
/**
* Get an option as an integer value from the source string.
* @param string $oname The option to check
* @param int $default Optional value to return if option is unset/blank.
* @return int
*/
function getIntOption( $oname, $default=0 ) {
$val = $this->getOption( $oname );
if( $val == '' ) {
$val = $default;
}
return intval( $val );
}
function setOption( $oname, $val ) {
$this->loadFromDatabase();
@ -1076,6 +1090,11 @@ class User {
# Clear cached skin, so the new one displays immediately in Special:Preferences
unset( $this->mSkin );
}
// Filter out any newlines that may have passed through input validation.
// Newlines are used to separate items in the options blob.
$val = str_replace( "\r\n", "\n", $val );
$val = str_replace( "\r", "\n", $val );
$val = str_replace( "\n", " ", $val );
$this->mOptions[$oname] = $val;
$this->invalidateCache();
}