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( '&nbsp;', ' ', $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="./">&laquo;</a> <?php echo $titleSrcFilename; ?></h1>
+<h1><a href="./">&laquo;</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> &bull; <a 
href="https://gerrit.wikimedia.org/r/gitweb?p=operations/mediawiki-config.git;a=blame;f=<?php
 echo $urlSrcFilename; ?>;hb=HEAD">blame</a> &bull; <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> &bull; <a 
href="https://gerrit.wikimedia.org/r/gitweb?p=operations/mediawiki-config.git;a=blame;f=<?php
 echo $selectedFileRepoPathEsc; ?>;hb=HEAD">blame</a> &bull; <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

Reply via email to