]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/commitdiff
Update img plugin to version 3.20160506
authorSimon McVittie <smcv@debian.org>
Wed, 4 May 2016 07:52:40 +0000 (08:52 +0100)
committerSimon McVittie <smcv@debian.org>
Sun, 8 May 2016 14:35:23 +0000 (15:35 +0100)
* 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
debian/changelog

index b92e24cc06fb93d8d88adcf1517903e46e49dc25..c3dc41474fed70094ba2837259b83fe76413b8a2 100644 (file)
@@ -21,6 +21,28 @@ sub getsetup () {
                        rebuild => undef,
                        section => "widget",
                },
                        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 (@) {
 }
 
 sub preprocess (@) {
@@ -64,52 +86,129 @@ sub preprocess (@) {
 
        my $dir = $params{page};
        my $base = IkiWiki::basename($file);
 
        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 $imglink;
-       my $r = $im->Read($srcfile);
-       error sprintf(gettext("failed to read %s: %s"), $file, $r) if $r;
-       
+       my $imgdatalink;
        my ($dwidth, $dheight);
 
        my ($dwidth, $dheight);
 
+       my ($w, $h);
        if ($params{size} ne 'full') {
        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);
                                ($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");
                                $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)) {
                        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 {
                                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;
 
                                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}) {
                                # 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 {
                                        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");
                        $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);
        }
 
        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";
 
        if (! exists $params{class}) {
                $params{class}="img";
@@ -168,10 +263,10 @@ sub preprocess (@) {
                }
        }
        
                }
        }
        
-       my $imgtag='<img src="'.$imgurl.
-               '" width="'.$dwidth.
-               '" height="'.$dheight.'"'.
-               $attrs.
+       my $imgtag='<img src="'.$imgurl.'"';
+       $imgtag.=' width="'.$dwidth.'"' if defined $dwidth;
+       $imgtag.=' height="'.$dheight.'"' if defined $dheight;
+       $imgtag.= $attrs.
                (exists $params{align} && ! exists $params{caption} ? ' align="'.$params{align}.'"' : '').
                ' />';
 
                (exists $params{align} && ! exists $params{caption} ? ' align="'.$params{align}.'"' : '').
                ' />';
 
index 919814f2f8f01f7084a700874a19e5dd208c2fdb..f0e6177310e51d127bdd0edbf8cb7d9326809148 100644 (file)
@@ -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)
 
   * 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 <smcv@debian.org>  Sun, 08 May 2016 15:33:51 +0100
 
 
  -- Simon McVittie <smcv@debian.org>  Sun, 08 May 2016 15:33:51 +0100