CSteipp has uploaded a new change for review.
https://gerrit.wikimedia.org/r/110069
Change subject: SECURITY: Sanitize shell command args
......................................................................
SECURITY: Sanitize shell command args
Add validation and sanitization to several code paths.
Bug: 60339
Change-Id: Id124281d21ec730a2e0bbace843dd97194a712b4
---
M includes/media/Bitmap.php
M includes/media/DjVu.php
M includes/media/ImageHandler.php
3 files changed, 37 insertions(+), 31 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/69/110069/1
diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php
index e2444a1..79b0497 100644
--- a/includes/media/Bitmap.php
+++ b/includes/media/Bitmap.php
@@ -277,27 +277,27 @@
$wgMaxAnimatedGifArea,
$wgImageMagickTempDir, $wgImageMagickConvertCommand;
- $quality = '';
- $sharpen = '';
+ $quality = array();
+ $sharpen = array();
$scene = false;
- $animation_pre = '';
- $animation_post = '';
- $decoderHint = '';
+ $animation_pre = array();
+ $animation_post = array();
+ $decoderHint = array();
if ( $params['mimeType'] == 'image/jpeg' ) {
- $quality = "-quality 80"; // 80%
+ $quality = array( '-quality', '80' ); // 80%
# Sharpening, see bug 6193
if ( ( $params['physicalWidth'] +
$params['physicalHeight'] )
/ ( $params['srcWidth'] +
$params['srcHeight'] )
< $wgSharpenReductionThreshold ) {
- $sharpen = "-sharpen " . wfEscapeShellArg(
$wgSharpenParameter );
+ $sharpen = array( '-sharpen',
$wgSharpenParameter );
}
if ( version_compare( $this->getMagickVersion(),
"6.5.6" ) >= 0 ) {
// JPEG decoder hint to reduce memory,
available since IM 6.5.6-2
- $decoderHint = "-define
jpeg:size={$params['physicalDimensions']}";
+ $decoderHint = array( '-define',
"jpeg:size={$params['physicalDimensions']}" );
}
} elseif ( $params['mimeType'] == 'image/png' ) {
- $quality = "-quality 95"; // zlib 9, adaptive filtering
+ $quality = array( '-quality', '95' ); // zlib 9,
adaptive filtering
} elseif ( $params['mimeType'] == 'image/gif' ) {
if ( $this->getImageArea( $image ) >
$wgMaxAnimatedGifArea ) {
@@ -307,15 +307,15 @@
} elseif ( $this->isAnimatedImage( $image ) ) {
// Coalesce is needed to scale animated GIFs
properly (bug 1017).
- $animation_pre = '-coalesce';
+ $animation_pre = array( '-coalesce' );
// We optimize the output, but -optimize is
broken,
// use optimizeTransparency instead (bug 11822)
if ( version_compare(
$this->getMagickVersion(), "6.3.5" ) >= 0 ) {
- $animation_post = '-fuzz 5% -layers
optimizeTransparency';
+ $animation_post = array( '-fuzz', '5%',
'-layers', 'optimizeTransparency' );
}
}
} elseif ( $params['mimeType'] == 'image/x-xcf' ) {
- $animation_post = '-layers merge';
+ $animation_post = array( '-layers', 'merge' );
}
// Use one thread only, to avoid deadlock bugs on OOM
@@ -327,26 +327,28 @@
$rotation = $this->getRotation( $image );
list( $width, $height ) = $this->extractPreRotationDimensions(
$params, $rotation );
- $cmd =
- wfEscapeShellArg( $wgImageMagickConvertCommand ) .
+ $cmd = call_user_func_array( 'wfEscapeShellArg', array_merge(
+ array( $wgImageMagickConvertCommand ),
+ $quality,
// Specify white background color, will be used for
transparent images
// in Internet Explorer/Windows instead of default
black.
- " {$quality} -background white" .
- " {$decoderHint} " .
- wfEscapeShellArg( $this->escapeMagickInput(
$params['srcPath'], $scene ) ) .
- " {$animation_pre}" .
+ array( '-background', 'white' ),
+ $decoderHint,
+ array( $this->escapeMagickInput( $params['srcPath'],
$scene ) ),
+ $animation_pre,
// For the -thumbnail option a "!" is needed to force
exact size,
// or ImageMagick may decide your ratio is wrong and
slice off
// a pixel.
- " -thumbnail " . wfEscapeShellArg(
"{$width}x{$height}!" ) .
+ array( '-thumbnail', "{$width}x{$height}!" ),
// Add the source url as a comment to the thumb, but
don't add the flag if there's no comment
( $params['comment'] !== ''
- ? " -set comment " . wfEscapeShellArg(
$this->escapeMagickProperty( $params['comment'] ) )
- : '' ) .
- " -depth 8 $sharpen " .
- " -rotate -$rotation " .
- " {$animation_post} " .
- wfEscapeShellArg( $this->escapeMagickOutput(
$params['dstPath'] ) );
+ ? array( '-set', 'comment',
$this->escapeMagickProperty( $params['comment'] ) )
+ : array() ),
+ array( '-depth', 8 ),
+ $sharpen,
+ array( '-rotate', "-$rotation" ),
+ $animation_post,
+ array( $this->escapeMagickOutput( $params['dstPath'] )
) ) );
wfDebug( __METHOD__ . ": running ImageMagick: $cmd\n" );
wfProfileIn( 'convert' );
@@ -456,8 +458,8 @@
$dst = wfEscapeShellArg( $params['dstPath'] );
$cmd = $wgCustomConvertCommand;
$cmd = str_replace( '%s', $src, str_replace( '%d', $dst, $cmd )
); # Filenames
- $cmd = str_replace( '%h', $params['physicalHeight'],
- str_replace( '%w', $params['physicalWidth'], $cmd ) );
# Size
+ $cmd = str_replace( '%h', wfEscapeShellArg(
$params['physicalHeight'] ),
+ str_replace( '%w', wfEscapeShellArg(
$params['physicalWidth'] ), $cmd ) ); # Size
wfDebug( __METHOD__ . ": Running custom convert command $cmd\n"
);
wfProfileIn( 'convert' );
$retval = 0;
@@ -744,7 +746,7 @@
case 'im':
$cmd = wfEscapeShellArg(
$wgImageMagickConvertCommand ) . " " .
wfEscapeShellArg(
$this->escapeMagickInput( $params['srcPath'], $scene ) ) .
- " -rotate -$rotation " .
+ " -rotate " . wfEscapeShellArg(
"-$rotation" ) . " " .
wfEscapeShellArg(
$this->escapeMagickOutput( $params['dstPath'] ) );
wfDebug( __METHOD__ . ": running ImageMagick:
$cmd\n" );
wfProfileIn( 'convert' );
diff --git a/includes/media/DjVu.php b/includes/media/DjVu.php
index b9e89d9..9b8116e 100644
--- a/includes/media/DjVu.php
+++ b/includes/media/DjVu.php
@@ -177,9 +177,12 @@
$srcPath = $image->getLocalRefPath();
# Use a subshell (brackets) to aggregate stderr from both
pipeline commands
# before redirecting it to the overall stdout. This works in
both Linux and Windows XP.
- $cmd = '(' . wfEscapeShellArg( $wgDjvuRenderer ) . "
-format=ppm -page={$page}" .
- "
-size={$params['physicalWidth']}x{$params['physicalHeight']} " .
- wfEscapeShellArg( $srcPath );
+ $cmd = '(' . wfEscapeShellArg(
+ $wgDjvuRenderer,
+ "-format=ppm",
+ "-page={$page}",
+
"-size={$params['physicalWidth']}x{$params['physicalHeight']}",
+ $srcPath );
if ( $wgDjvuPostProcessor ) {
$cmd .= " | {$wgDjvuPostProcessor}";
}
diff --git a/includes/media/ImageHandler.php b/includes/media/ImageHandler.php
index e079003..6794e4b 100644
--- a/includes/media/ImageHandler.php
+++ b/includes/media/ImageHandler.php
@@ -93,6 +93,7 @@
if ( !isset( $params['page'] ) ) {
$params['page'] = 1;
} else {
+ $params['page'] = intval( $params['page'] );
if ( $params['page'] > $image->pageCount() ) {
$params['page'] = $image->pageCount();
}
--
To view, visit https://gerrit.wikimedia.org/r/110069
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id124281d21ec730a2e0bbace843dd97194a712b4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_22
Gerrit-Owner: CSteipp <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
