Krinkle has uploaded a new change for review. https://gerrit.wikimedia.org/r/59034
Change subject: noc: Refactor highlight.php to be simpler and <s>less</s> more secure ...................................................................... noc: Refactor highlight.php to be simpler and <s>less</s> more secure This is a second take at what I started 8 months ago (before this was in versio control) and followed-up on in I2d8a2a0ca8 but was partially reverted in Idcec674abd due to bug 47112. The main point of this refactor was to not mix logic with symlinks and auto-guessting path destinations. Instead glob the whitelist (the conf directory with symlinks) and only use that. This was successfully implemented, but when I cleaned it up in I2d8a2a0ca8 I made a stupid typo of a variable name: The foreach loop variable $viewFilename of $viewFilenames conflicted with the equally-named variable outside the loop that was supposed to represent the sanitised version that is considered safe and in the whitelist. As a result bug 47112. I've now re-implemented this (without the typo, obviously) and made it even stricter by using the whitelist exclusively for any resolution. Not just to filter the user input, but also to open the file (by reading the symlink). This completely gets rid the logic that tries to find the file in /home/wikipedia/common, which is why previously a simple typo could cause bug 47112. Also picked more descriptive variable names. Change-Id: I56b455ebb8e3e083bb6390e03e350b69b902bc64 --- M docroot/noc/conf/highlight.php 1 file changed, 48 insertions(+), 41 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/operations/mediawiki-config refs/changes/34/59034/1 diff --git a/docroot/noc/conf/highlight.php b/docroot/noc/conf/highlight.php index 05553be..4c68601 100644 --- a/docroot/noc/conf/highlight.php +++ b/docroot/noc/conf/highlight.php @@ -1,19 +1,29 @@ <?php // Only allow viewing of files of which there is a copy (or link) // in noc/conf/* by the same name. -$viewFilenames = array_map( 'basename', glob( __DIR__ . '/*' ) ); +$selectableFilepaths = glob( __DIR__ . '/*' ); -$srcFilename = $_GET['file']; +// Name of file from user input +$selectedFileName = $_GET['file']; -$viewFilename = false; -$srcDir = false; +// Absolute path to file on disk +$selectedFilePath = false; -foreach ( $viewFilenames as $view ) { - $src = substr( $view, -4 ) === '.txt' - ? substr( $view, 0, -4 ) - : $view; - if ( $srcFilename === $src ) { - $viewFilename = $view; +// Path to file in mediawiki-config repository +$selectedFileRepoDirName = false; +$selectedFileRepoPath = false; + +// Relative path to the symlink in conf/* +$selectedFileViewRawUrl = false; + +foreach ( $selectableFilepaths as $filePath ) { + $fileName = basename( $filePath ); + // Map .txt links to the original filename + if ( substr( $fileName, -4 ) === '.txt' ) { + $fileName = substr( $fileName, 0, -4 ); + } + if ( $fileName === $selectedFileName ) { + $selectedFilePath = $filePath; break; } } @@ -21,61 +31,58 @@ ob_start( 'ob_gzhandler' ); header( 'Content-Type: text/html; charset=utf-8' ); -if ( !$viewFilename ) { - # Secret site password distribution :-D - # First implement access control - if ( isset( $_SERVER['HTTP_REFERER'] ) && strpos( $_SERVER['HTTP_REFERER'], 'google' ) !== false ) { - header( "HTTP/1.1 404 Not Found" ); +if ( !$selectedFilePath ) { + header( "HTTP/1.1 404 Not Found" ); + if ( isset( $_SERVER['HTTP_REFERER'] ) && strpos( strtolower( $_SERVER['HTTP_REFERER'] ), 'google' ) !== false ) { echo "File not found\n"; exit; } - # OK, authenticated developer, send password + // Easter egg $hlHtml = highlight_string( '<'."?php\n\$secretSitePassword = 'jgmeidj28gms';\n", true ); + } else { - $baseSrcDir = '/home/wikipedia/common'; + // Follow symlink + if ( !file_exists( $selectedFilePath ) ) { + $hlHtml = 'Whitelist contains inexistant entry. :('; + } elseif ( !is_link( $selectedFilePath ) ) { + $hlHtml = 'Whitelist must only contain symlinks.'; + } else { + $selectedFileViewRawUrl = './' . urlencode( basename( $selectedFilePath ) ); + // Resolve symlink + $selectedFilePath = readlink( $selectedFilePath ); + // Figure out path to selected file in the mediawiki-config repository + $selectedFileRepoPath = ( dirname( $selectedFilePath ) === 'wmf-config' ? 'wmf-config/' : '' ) . $selectedFileName; - // Find where it is - if ( file_exists( "$baseSrcDir/wmf-config/$srcFilename" ) ) { - $srcPath = "$baseSrcDir/wmf-config/$srcFilename"; - $srcDir = 'wmf-config/'; - } elseif ( file_exists( "$baseSrcDir/$srcFilename" ) ) { - $srcPath = "$baseSrcDir/$srcFilename"; - $srcDir = ''; - } + if ( substr( $selectedFileName, -4 ) === '.php' ) { - // How to display it - if ( $srcDir !== false ) { - if ( substr( $srcFilename, -4 ) === '.php' ) { - - $hlHtml = highlight_file( $srcPath, true ); + $hlHtml = highlight_file( $selectedFilePath, true ); $hlHtml = str_replace( ' ', ' ', $hlHtml ); // https://bugzilla.wikimedia.org/19253 $hlHtml = str_replace( ' ', "\t", $hlHtml ); // convert 4 spaces to 1 tab character; bug #36576 } else { - $hlHtml = htmlspecialchars( file_get_contents( __DIR__ . '/' . $srcFilename ) ); + $hlHtml = htmlspecialchars( file_get_contents( $selectedFilePath ) ); } - } else { - $hlHtml = 'Failed to read file. :('; } $hlHtml = "<pre>$hlHtml</pre>"; } -$titleSrcFilename = htmlspecialchars( $srcFilename ); -$urlSrcFilename = htmlspecialchars( urlencode( $srcDir . $srcFilename ) ); -$urlViewFilename = htmlspecialchars( urlencode( $viewFilename ) ); +$selectedFileNameEsc = htmlspecialchars( $selectedFileName ); +$selectedFileRepoPathEsc = htmlspecialchars( $selectedFileRepoPath ); +$selectedFileViewRawUrlEsc = htmlspecialchars( $selectedFileViewRawUrl ); ?> <!DOCTYPE html> <html lang="en"> <head> - <title><?php echo $titleSrcFilename; ?></title> - <link rel="stylesheet" href="/base.css"> + <title><?php echo $selectedFileNameEsc; ?> - Wikimedia configuration files</title> + <link rel="shortcut icon" href="//bits.wikimedia.org/favicon/wmf.ico"> + <link rel="stylesheet" href="../base.css"> </head> <body> -<h1><a href="./">«</a> <?php echo $titleSrcFilename; ?></h1> +<h1><a href="./">«</a> <?php echo $selectedFileNameEsc; ?></h1> <?php -if ( $srcDir !== false ) : +if ( $selectedFilePath !== false ) : ?> -<p>(<a href="https://gerrit.wikimedia.org/r/gitweb?p=operations/mediawiki-config.git;a=history;f=<?php echo $urlSrcFilename; ?>;hb=HEAD">version control</a> • <a href="https://gerrit.wikimedia.org/r/gitweb?p=operations/mediawiki-config.git;a=blame;f=<?php echo $urlSrcFilename; ?>;hb=HEAD">blame</a> • <a href="./<?php echo $urlViewFilename; ?>">raw text</a>)</p> +<p>(<a href="https://gerrit.wikimedia.org/r/gitweb?p=operations/mediawiki-config.git;a=history;f=<?php echo $selectedFileRepoPathEsc; ?>;hb=HEAD">version control</a> • <a href="https://gerrit.wikimedia.org/r/gitweb?p=operations/mediawiki-config.git;a=blame;f=<?php echo $selectedFileRepoPathEsc; ?>;hb=HEAD">blame</a> • <a href="<?php echo $selectedFileViewRawUrlEsc; ?>">raw text</a>)</p> <?php endif; ?> -- To view, visit https://gerrit.wikimedia.org/r/59034 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I56b455ebb8e3e083bb6390e03e350b69b902bc64 Gerrit-PatchSet: 1 Gerrit-Project: operations/mediawiki-config Gerrit-Branch: master Gerrit-Owner: Krinkle <krinklem...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits