img: check magic number before giving common formats to ImageMagick

This mitigates CVE-2016-3714 and similar vulnerabilities by
avoiding passing obviously-wrong input to ImageMagick decoders.

diff --git a/IkiWiki/Plugin/img.pm b/IkiWiki/Plugin/img.pm
index 53d963425eb1dade14cdb5e8406457b20b2916f7..ed2e9354da988dfdfe1894d1fe6d7bb93e1ad4c1 100644 (file)
--- a/IkiWiki/Plugin/img.pm
+++ b/IkiWiki/Plugin/img.pm
@@ -99,20 +99,29 @@ sub preprocess (@) {
        # Never interpret well-known file extensions as any other format,
        # in case the wiki configuration unwisely allows attaching
        # arbitrary files named *.jpg, etc.
        # 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';
        if ($extension =~ m/^(jpeg|jpg)$/is) {
                $format = 'jpeg';

+               $magic = "\377\330\377";

        }
        elsif ($extension =~ m/^(png)$/is) {
                $format = 'png';
        }
        elsif ($extension =~ m/^(png)$/is) {
                $format = 'png';

+               $magic = "\211PNG\r\n\032\n";

        }
        elsif ($extension =~ m/^(gif)$/is) {
                $format = 'gif';
        }
        elsif ($extension =~ m/^(gif)$/is) {
                $format = 'gif';

+               $magic = "GIF8";

        }
        elsif ($extension =~ m/^(svg)$/is) {
                $format = 'svg';
        }
        elsif ($extension =~ m/^(pdf)$/is) {
                $format = 'pdf';
        }
        elsif ($extension =~ m/^(svg)$/is) {
                $format = 'svg';
        }
        elsif ($extension =~ m/^(pdf)$/is) {
                $format = 'pdf';

+               $magic = "%PDF-";

        }
        else {
                # allow ImageMagick to auto-detect (potentially dangerous)
        }
        else {
                # allow ImageMagick to auto-detect (potentially dangerous)


@@ -121,6 +130,25 @@ sub preprocess (@) {
 
        error sprintf(gettext("%s image processing disabled in img_allowed_formats configuration"), $format ? $format : "\"$extension\"") unless allowed($format ? $format : "everything");
 
 
        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 ($format eq 'svg') {
+               my $content;
+               read($in, $content, 5) or error sprintf(gettext("failed to read %s: %s"), $file, $!);
+               # This is an over-simplification, but ?xml is the check that
+               # ImageMagick uses. We also accept <svg for the simplest
+               # possible SVGs.
+               if ($content !~ m/^(.\?xml|<svg)/is) {
+                       error sprintf(gettext("\"%s\" does not seem to be a valid %s file"), $file, $format);
+               }
+       }
+       elsif ($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 $issvg = $base=~s/\.svg$/.png/i;
        my $ispdf = $base=~s/\.pdf$/.png/i;
        my $pagenumber = exists($params{pagenumber}) ? int($params{pagenumber}) : 0;
        my $issvg = $base=~s/\.svg$/.png/i;
        my $ispdf = $base=~s/\.pdf$/.png/i;
        my $pagenumber = exists($params{pagenumber}) ? int($params{pagenumber}) : 0;

diff --git a/t/img.t b/t/img.t
index 3d86db6baaf5672faddaa5f4a0f548837583a19f..5d0e0f09cdbc127eb264f99e0aae7645d1a92a09 100755 (executable)
--- a/t/img.t
+++ b/t/img.t
@@ -145,10 +145,15 @@ like($outhtml, qr{src="(\./)?imgconversions/11x-redsquare\.jpg" width="11" heigh
 like($outhtml, qr{src="(\./)?imgconversions/12x-redsquare\.jpeg" width="12" height="12"});
 
 # We do not misinterpret images
 like($outhtml, qr{src="(\./)?imgconversions/12x-redsquare\.jpeg" width="12" height="12"});
 
 # We do not misinterpret images

+my $quot = qr/(?:"|")/;
+like($outhtml, qr/${quot}really-svg\.png${quot} does not seem to be a valid png file/);

 ok(! -e "$outpath/666x-really-svg.png");
 ok(! -e "$outpath/666x-really-svg.bmp");
 ok(! -e "$outpath/666x-really-svg.png");
 ok(! -e "$outpath/666x-really-svg.bmp");

+like($outhtml, qr/${quot}really-pdf\.jpeg${quot} does not seem to be a valid jpeg file/);

 ok(! -e "$outpath/666x-really-pdf.jpeg");
 ok(! -e "$outpath/666x-really-pdf.jpeg");

+like($outhtml, qr/${quot}really-pdf\.jpg${quot} does not seem to be a valid jpeg file/);

 ok(! -e "$outpath/666x-really-pdf.jpg");
 ok(! -e "$outpath/666x-really-pdf.jpg");

+like($outhtml, qr/${quot}really-pdf\.png${quot} does not seem to be a valid png file/);

 ok(! -e "$outpath/666x-really-pdf.png");
 
 # disable support for uncommon formats and try again
 ok(! -e "$outpath/666x-really-pdf.png");
 
 # disable support for uncommon formats and try again