From b84854289f508404f15612328b89ad0627569f49 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 4 May 2016 08:52:40 +0100 Subject: [PATCH] Update img plugin to version 3.20160506 * Update img plugin to version 3.20160506 to mitigate ImageMagick vulnerabilities, including remote code execution (CVE-2016-3714): - Never convert SVG images to PNG; simply pass them through to the browser. This prevents exploitation of any ImageMagick SVG coder vulnerabilities. (joeyh) - Do not resize image formats other than JPEG, PNG, GIF unless specifically configured to do so. This prevents exploitation of any vulnerabilities in less common coders, such as MVG. (smcv) - Do not resize JPEG, PNG, GIF, PDF images if their extensions do not match their "magic numbers", because wiki admins might try to restrict attachments by extension, but ImageMagick can base its choice of coder on the magic number. Explicitly force the obvious ImageMagick coder to be used. (smcv) * Minor non-security changes resulting from that update, since reverting them seems higher-risk than keeping them: - Add PDF support, disabled by the above changes unless specifically configured (chrysn) - Only render one frame or page from animated GIF or multi-page PDF (chrysn) - Do not distort aspect ratio when resizing small images (chrysn) - Use data: URLs to embed images in page previews (chrysn) - Raise an error if the image's size cannot be determined (chrysn) - Handle filenames containing a colon correctly (smcv) --- IkiWiki/Plugin/img.pm | 217 ++++++++++++++++++++++++++++++------------ debian/changelog | 23 +++++ 2 files changed, 179 insertions(+), 61 deletions(-) diff --git a/IkiWiki/Plugin/img.pm b/IkiWiki/Plugin/img.pm index b92e24cc0..c3dc41474 100644 --- a/IkiWiki/Plugin/img.pm +++ b/IkiWiki/Plugin/img.pm @@ -21,6 +21,28 @@ sub getsetup () { rebuild => undef, section => "widget", }, + img_allowed_formats => { + type => "string", + default => [qw(jpeg png gif svg)], + description => "Image formats to process (jpeg, png, gif, svg, pdf or 'everything' to accept all)", + # ImageMagick has had arbitrary code execution flaws, + # and the whole delegates mechanism is scary from + # that perspective + safe => 0, + rebuild => 0, + }, +} + +sub allowed { + my $format = shift; + my $allowed = $config{img_allowed_formats}; + $allowed = ['jpeg', 'png', 'gif', 'svg'] unless defined $allowed && @$allowed; + + foreach my $a (@$allowed) { + return 1 if lc($a) eq $format || lc($a) eq 'everything'; + } + + return 0; } sub preprocess (@) { @@ -64,52 +86,129 @@ sub preprocess (@) { my $dir = $params{page}; my $base = IkiWiki::basename($file); - my $issvg = $base=~s/\.svg$/.png/i; + my $extension; + my $format; + + if ($base =~ m/\.([a-z0-9]+)$/) { + $extension = $1; + } + else { + error gettext("Unable to detect image type from extension"); + } + + # Never interpret well-known file extensions as any other format, + # in case the wiki configuration unwisely allows attaching + # arbitrary files named *.jpg, etc. + my $magic; + my $offset = 0; + open(my $in, '<', $srcfile) or error sprintf(gettext("failed to read %s: %s"), $file, $!); + binmode($in); + + if ($extension =~ m/^(jpeg|jpg)$/is) { + $format = 'jpeg'; + $magic = "\377\330\377"; + } + elsif ($extension =~ m/^(png)$/is) { + $format = 'png'; + $magic = "\211PNG\r\n\032\n"; + } + elsif ($extension =~ m/^(gif)$/is) { + $format = 'gif'; + $magic = "GIF8"; + } + elsif ($extension =~ m/^(svg)$/is) { + $format = 'svg'; + } + elsif ($extension =~ m/^(pdf)$/is) { + $format = 'pdf'; + $magic = "%PDF-"; + } + else { + # allow ImageMagick to auto-detect (potentially dangerous) + $format = ''; + } + + error sprintf(gettext("%s image processing disabled in img_allowed_formats configuration"), $format ? $format : "\"$extension\"") unless allowed($format ? $format : "everything"); + + # Try harder to protect ImageMagick from itself + if (defined $magic) { + my $content; + read($in, $content, length $magic) or error sprintf(gettext("failed to read %s: %s"), $file, $!); + if ($magic ne $content) { + error sprintf(gettext("\"%s\" does not seem to be a valid %s file"), $file, $format); + } + } + + my $ispdf = $base=~s/\.pdf$/.png/i; + my $pagenumber = exists($params{pagenumber}) ? int($params{pagenumber}) : 0; + if ($pagenumber != 0) { + $base = "p$pagenumber-$base"; + } - eval q{use Image::Magick}; - error gettext("Image::Magick is not installed") if $@; - my $im = Image::Magick->new($issvg ? (magick => "png") : ()); my $imglink; - my $r = $im->Read($srcfile); - error sprintf(gettext("failed to read %s: %s"), $file, $r) if $r; - + my $imgdatalink; my ($dwidth, $dheight); + my ($w, $h); if ($params{size} ne 'full') { - my ($w, $h) = ($params{size} =~ /^(\d*)x(\d*)$/); - error sprintf(gettext('wrong size format "%s" (should be WxH)'), $params{size}) - unless (defined $w && defined $h && - (length $w || length $h)); - - if ((length $w && $w > $im->Get("width")) || - (length $h && $h > $im->Get("height"))) { - # resizing larger - $imglink = $file; + ($w, $h) = ($params{size} =~ /^(\d*)x(\d*)$/); + } - # don't generate larger image, just set display size - if (length $w && length $h) { - ($dwidth, $dheight)=($w, $h); - } - # avoid division by zero on 0x0 image - elsif ($im->Get("width") == 0 || $im->Get("height") == 0) { + if ($format eq 'svg') { + # svg images are not scaled using ImageMagick because the + # pipeline is complex. Instead, the image size is simply + # set to the provided values. + # + # Aspect ratio will be preserved automatically when + # only a width or only a height is specified. + # When both are specified, aspect ratio will not be + # preserved. + $imglink = $file; + $dwidth = $w if length $w; + $dheight = $h if length $h; + } + else { + eval q{use Image::Magick}; + error gettext("Image::Magick is not installed") if $@; + my $im = Image::Magick->new(); + my $r = $im->Read("$format:$srcfile\[$pagenumber]"); + error sprintf(gettext("failed to read %s: %s"), $file, $r) if $r; + + if (! defined $im->Get("width") || ! defined $im->Get("height")) { + error sprintf(gettext("failed to get dimensions of %s"), $file); + } + + if (! length $w && ! length $h) { + $dwidth = $im->Get("width"); + $dheight = $im->Get("height"); + } else { + error sprintf(gettext('wrong size format "%s" (should be WxH)'), $params{size}) + unless (defined $w && defined $h && + (length $w || length $h)); + + if ($im->Get("width") == 0 || $im->Get("height") == 0) { ($dwidth, $dheight)=(0, 0); - } - # calculate unspecified size from the other one, preserving - # aspect ratio - elsif (length $w) { - $dwidth=$w; - $dheight=$w / $im->Get("width") * $im->Get("height"); - } - elsif (length $h) { + } elsif (! length $w || (length $h && $im->Get("height")*$w > $h * $im->Get("width"))) { + # using height because only height is given or ... + # because original image is more portrait than $w/$h + # ... slimness of $im > $h/w + # ... $im->Get("height")/$im->Get("width") > $h/$w + # ... $im->Get("height")*$w > $h * $im->Get("width") + $dheight=$h; $dwidth=$h / $im->Get("height") * $im->Get("width"); + } else { # (! length $h) or $w is what determines the resized size + $dwidth=$w; + $dheight=$w / $im->Get("width") * $im->Get("height"); } } - else { - # resizing smaller - my $outfile = "$config{destdir}/$dir/${w}x${h}-$base"; - $imglink = "$dir/${w}x${h}-$base"; - + + if ($dwidth < $im->Get("width") || $ispdf) { + # resize down, or resize to pixels at all + + my $outfile = "$config{destdir}/$dir/$params{size}-$base"; + $imglink = "$dir/$params{size}-$base"; + will_render($params{page}, $imglink); if (-e $outfile && (-M $srcfile >= -M $outfile)) { @@ -118,44 +217,40 @@ sub preprocess (@) { error sprintf(gettext("failed to read %s: %s"), $outfile, $r) if $r; } else { - $r = $im->Resize(geometry => "${w}x${h}"); + $r = $im->Resize(geometry => "${dwidth}x${dheight}"); error sprintf(gettext("failed to resize: %s"), $r) if $r; + $im->set($ispdf ? (magick => 'png') : ()); + my @blob = $im->ImageToBlob(); # don't actually write resized file in preview mode; # rely on width and height settings if (! $params{preview}) { - my @blob = $im->ImageToBlob(); writefile($imglink, $config{destdir}, $blob[0], 1); } else { - $imglink = $file; + eval q{use MIME::Base64}; + error($@) if $@; + $imgdatalink = "data:image/".$im->Get("magick").";base64,".encode_base64($blob[0]); } } - # always get the true size of the resized image - $dwidth = $im->Get("width"); + # always get the true size of the resized image (it could be + # that imagemagick did its calculations differently) + $dwidth = $im->Get("width"); $dheight = $im->Get("height"); + } else { + $imglink = $file; + } + + if (! defined($dwidth) || ! defined($dheight)) { + error sprintf(gettext("failed to determine size of image %s"), $file) } - } - else { - $imglink = $file; - $dwidth = $im->Get("width"); - $dheight = $im->Get("height"); - } - - if (! defined($dwidth) || ! defined($dheight)) { - error sprintf(gettext("failed to determine size of image %s"), $file) } my ($fileurl, $imgurl); - if (! $params{preview}) { - $fileurl=urlto($file, $params{destpage}); - $imgurl=urlto($imglink, $params{destpage}); - } - else { - $fileurl=urlto($file); - $imgurl=urlto($imglink); - } + my $urltobase = $params{preview} ? undef : $params{destpage}; + $fileurl=urlto($file, $urltobase); + $imgurl=$imgdatalink ? $imgdatalink : urlto($imglink, $urltobase); if (! exists $params{class}) { $params{class}="img"; @@ -168,10 +263,10 @@ sub preprocess (@) { } } - my $imgtag=''; diff --git a/debian/changelog b/debian/changelog index 919814f2f..f0e617731 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,29 @@ ikiwiki (3.20120629.3) UNRELEASED; urgency=medium * HTML-escape error messages, in one case avoiding potential cross-site scripting (CVE-2016-4561, OVE-20160505-0012) + * Update img plugin to version 3.20160506 to mitigate ImageMagick + vulnerabilities, including remote code execution (CVE-2016-3714): + - Never convert SVG images to PNG; simply pass them through to the + browser. This prevents exploitation of any ImageMagick SVG coder + vulnerabilities. (joeyh) + - Do not resize image formats other than JPEG, PNG, GIF unless + specifically configured to do so. This prevents exploitation + of any vulnerabilities in less common coders, such as MVG. (smcv) + - Do not resize JPEG, PNG, GIF, PDF images if their extensions do + not match their "magic numbers", because wiki admins might try to + restrict attachments by extension, but ImageMagick can base its + choice of coder on the magic number. Explicitly force the + obvious ImageMagick coder to be used. (smcv) + * Minor non-security changes resulting from that update, since + reverting them seems higher-risk than keeping them: + - Add PDF support, disabled by the above changes unless specifically + configured (chrysn) + - Only render one frame or page from animated GIF or multi-page PDF + (chrysn) + - Do not distort aspect ratio when resizing small images (chrysn) + - Use data: URLs to embed images in page previews (chrysn) + - Raise an error if the image's size cannot be determined (chrysn) + - Handle filenames containing a colon correctly (smcv) -- Simon McVittie Sun, 08 May 2016 15:33:51 +0100 -- 2.39.5