Undefined behaviors of casting double to size_t in svg, mvg and other coders (recurring bugs of CVE-2022-32546)

[fullwaywang]

ImageMagick version

7.1.30-0

Operating system

Linux

Operating system, version and so on

Any

Description

While reviewing historical vulnerabilities, I discovered several similar bugs just like issue #4985 which was later assigned CVE-2022-32546.

The original vulnerability relates to casting a width/height value in double to size_t or unsigned long, which denotes columns/rows number in pixels. The issue concerns about PCL format, and the fix (#4986 ) was about PCL only.

However, with a CodeQL rule, I am able to find almost the same issues in several other readers ReadXXXImage functions, like CAPTION, EMF, LABEL, MVG, PS, PS2, PS3, SVG, WMF. It was easy to construct sample mvg and svg files to trigger these bugs, as shown below.

Steps to Reproduce

Prerequisite

configure ImageMagick with UBSAN. If compiled with gcc, specifically turn on flow-cast-overflow:

./configure CFLAGS='-g -fsanitize=address,undefined,float-cast-overflow -fsanitize-recover=address,undefined,float-cast-overflow' LDFLAGS='-fsanitize=address,undefined,float-cast-overflow -lasan -lubsan'

Trigger

Use the malformed sample files in the attached zip. The following commands demonstrates the bugs.

/data/home/fullwaywang/exp
➜ magick identify bitcoin.svg
coders/svg.c:2829:38: runtime error: 5e+24 is outside the range of representable values of type 'long unsigned int'
bitcoin.svg SVG 0x234 0x234+0+0 16-bit sRGB 19056B 0.010u 0:00.015

/data/home/fullwaywang/exp
➜ magick identify mvg:piechart.mvg
coders/mvg.c:180:33: runtime error: 5e+26 is outside the range of representable values of type 'long unsigned int'
identify: MustSpecifyImageSize `piechart.mvg' @ error/mvg.c/ReadMVGImage/186.

Images

samples.zip

Reporter

fullwaywang

[urban-warrior]

Thanks for the problem report. We can reproduce it and will have a patch to fix it in the GIT main branch @ https://github.com/ImageMagick/ImageMagick later today. The patch will be available in the beta releases of ImageMagick @ https://imagemagick.org/archive/beta/ by sometime tomorrow.

[ByteHackr]

CVE-2023-34151 was assigned for this issue.

[syominsergey]

@urban-warrior Hello!

Recently I have tested presence of CVE-2023-34151 in actual code of ImageMagick and I think that bug was not properly closed. Maybe I am wrong. Please let me know, what do yo think about it.

Steps to reproduce.

Take vagrant image of debian bookworm from here.
Setup new virtual machine based on this image in vagrant (I used virtual box as provider).
I used this Vagrantfile:

# -*- mode: ruby -*-
# vi: set ft=ruby :

Vagrant.configure("2") do |config|
  config.vm.box = "debian/bookworm64 12.20240212.1"
  config.vm.provider "virtualbox" do |v|
    v.cpus = 4
    v.memory = 4096
  end
end

(I imported image of debian bookworm locally with name debian/bookworm64 12.20240212.1)
Start VM:

vagrant up

Login into VM with ssh:

vagrant ssh

Update VMs OS apt cache using sudo apt update. Upgradable packages you can see here.
And then sudo apt full-upgrade. You can see logs here.
Then exit from ssh, stop VM using vagrant halt, start it again using vagrant up and connect again with ssh using vagrant ssh.
Then install build dependencies for imagemagick. To make this process easier, I installed build dependencies for package imagemagick from debian bookworm in hope they will satisfy build dependencies of imagemagick from upstream:

sudo apt build-dep imagemagick

See logs of build deps installation here.
Then install additional packages using apt:

sudo apt install git unzip

(logs of installation of this packages you can see here)
Clone sources of ImageMagick from upstrem:

git clone https://github.com/ImageMagick/ImageMagick.git

Current commit in my case when I tested:

vagrant@bookworm:~/ImageMagick$ git log -1
commit 28f890a3ab0b934aabf737af3b44456e0c229e91 (HEAD -> main, origin/main, origin/HEAD)
Author: Florian Levis <levis.florian@gmail.com>
Date:   Thu Apr 18 07:45:08 2024 +0200

    Update release.yml (#7258)
    
    Remove unnecessary fuse requirements

Then run configure as it was defined in this issue:

vagrant@bookworm:~/ImageMagick$ time ./configure CFLAGS='-g -fsanitize=address,undefined,float-cast-overflow -fsanitize-recover=address,undefined,float-cast-overflow' LDFLAGS='-fsanitize=address,undefined,float-cast-overflow -lasan -lubsan' |& tee /vagrant/configure.log

You can see configuration logs here.
Then make imagemagick:

vagrant@bookworm:~/ImageMagick$ time make -j4 |& tee /vagrant/make.log

You can see make logs here.
Then download and unpack samples from current issue:

vagrant@bookworm:~$ mkdir -p CVE-2023-34151/samples
vagrant@bookworm:~$ cd CVE-2023-34151/samples
vagrant@bookworm:~/CVE-2023-34151/samples$ wget https://github.com/ImageMagick/ImageMagick/files/11497437/samples.zip
...
vagrant@bookworm:~/CVE-2023-34151/samples$ unzip samples.zip 
Archive:  samples.zip
  inflating: piechart.mvg            
  inflating: bitcoin.svg

And finally try to reproduce CVE using magick.sh wrapper:

vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify /home/vagrant/CVE-2023-34151/samples/bitcoin.svg
/home/vagrant/CVE-2023-34151/samples/bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.010u 0:00.010
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
coders/mvg.c:180:33: runtime error: 5e+26 is outside the range of representable values of type 'long unsigned int'
identify: must specify image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/mvg.c/ReadMVGImage/186.

I think, that this output shows that there is no problem with file bitcoin.svg. So current bug was fixed for svg. But invocation with piechart.mvg shows presence of bug for mvg.

@urban-warrior, please, let me know, what dou you think about this observation.

[syominsergey]

JFYI

After 5ab109d error message was little changed, but I think bug remains for mvg:

vagrant@bookworm:~/ImageMagick$ git log -1
commit 58e7ea218aa1852bcd103893e350d7951a07cde3 (HEAD -> main, origin/main, origin/HEAD)
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Fri Apr 19 09:10:39 2024 -0400

    latest documentation
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify /home/vagrant/CVE-2023-34151/samples/bitcoin.svg
/home/vagrant/CVE-2023-34151/samples/bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.000u 0:00.009
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
coders/mvg.c:194:48: runtime error: 1.84467e+19 is outside the range of representable values of type 'long unsigned int'
identify: negative or zero image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/image.c/SetImageExtent/2667.

[syominsergey]

After change like 5ab109d for other casts error transforms but remains:

vagrant@bookworm:~/ImageMagick$ git log -1
commit 58e7ea218aa1852bcd103893e350d7951a07cde3 (HEAD -> main, origin/main, origin/HEAD)
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Fri Apr 19 09:10:39 2024 -0400

    latest documentation
vagrant@bookworm:~/ImageMagick$ git diff coders/mvg.c
diff --git a/coders/mvg.c b/coders/mvg.c
index 762837550..13c223381 100644
--- a/coders/mvg.c
+++ b/coders/mvg.c
@@ -191,8 +191,8 @@ static Image *ReadMVGImage(const ImageInfo *image_info,ExceptionInfo *exception)
     96.0;
   draw_info->affine.sy=image->resolution.y == 0.0 ? 1.0 : image->resolution.y/
     96.0;
-  image->columns=(size_t) (draw_info->affine.sx*image->columns);
-  image->rows=(size_t) (draw_info->affine.sy*image->rows);
+  image->columns=CastDoubleToUnsigned(draw_info->affine.sx*image->columns);
+  image->rows=CastDoubleToUnsigned(draw_info->affine.sy*image->rows);
   status=SetImageExtent(image,image->columns,image->rows,exception);
   if (status == MagickFalse)
     {
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
MagickCore/image-private.h:124:3: runtime error: 1.84467e+19 is outside the range of representable values of type 'long unsigned int'
identify: negative or zero image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/image.c/SetImageExtent/2667.

[syominsergey]

It's interesting, that now, as I understand, problem is here:

ImageMagick/MagickCore/image-private.h

Line 124 in 58e7ea2

return((size_t) x);

I don't understand how to fix it in right way for now :(

[dlemstra]

That range error happens when x is higher than MAGICK_SIZE_MAX-1. There is a check to make sure that floor(x) is not higher than that maximum value but we are still using the "unfloored" value in the typecast. I think we should change the method to something like this (the typecast does a truncate and that is similar to a floor):

static inline size_t CastDoubleToUnsigned(const double x)
{
  double
    value;

  if (IsNaN(x) != 0)
    {
      errno=ERANGE;
      return(0);
    }
  value=floor(x);
  if (value > ((double) MAGICK_SIZE_MAX-1))
    {
      errno=ERANGE;
      return((size_t) MAGICK_SIZE_MAX);
    }
  if (value < 0.0)
    {
      errno=ERANGE;
      return(0);
    }
  return((size_t) value);
}

[syominsergey]

Unfortunately, problem remains after ae164b0 with mvg:

vagrant@bookworm:~/ImageMagick$ git log -1
commit ae164b0c76ac3a23dfdb5416c8882c3e936d652f (HEAD -> main, origin/main, origin/HEAD)
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Fri Apr 19 13:39:47 2024 -0400

    improved range checking (https://github.com/ImageMagick/ImageMagick/issues/6341)
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
MagickCore/image-private.h:134:3: runtime error: 1.84467e+19 is outside the range of representable values of type 'long unsigned int'
identify: negative or zero image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/image.c/SetImageExtent/2667.

It is still here:

ImageMagick/MagickCore/image-private.h

Line 134 in ae164b0

return((size_t) x);

[syominsergey]

Using CastDoubleToUnsigned from here #6341 (comment) bug also remains:

vagrant@bookworm:~/ImageMagick$ git log -1
commit ae164b0c76ac3a23dfdb5416c8882c3e936d652f (HEAD -> main, origin/main, origin/HEAD)
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Fri Apr 19 13:39:47 2024 -0400

    improved range checking (https://github.com/ImageMagick/ImageMagick/issues/6341)
vagrant@bookworm:~/ImageMagick$ git diff MagickCore/image-private.h
diff --git a/MagickCore/image-private.h b/MagickCore/image-private.h
index 86637f30f..2f6d6ded6 100644
--- a/MagickCore/image-private.h
+++ b/MagickCore/image-private.h
@@ -125,13 +125,12 @@ static inline size_t CastDoubleToUnsigned(const double x)
       errno=ERANGE;
       return((size_t) MAGICK_SIZE_MAX);
     }
-  value=ceil(x);
   if (value < 0.0)
     {
       errno=ERANGE;
       return(0);
     }
-  return((size_t) x);
+  return((size_t) value);
 }
 
 static inline double DegreesToRadians(const double degrees)
vagrant@bookworm:~/ImageMagick$ make -j4
make  all-am
make[1]: Entering directory '/home/vagrant/ImageMagick'
make[1]: Nothing to be done for 'all-am'.
make[1]: Leaving directory '/home/vagrant/ImageMagick'
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
MagickCore/image-private.h:133:3: runtime error: 1.84467e+19 is outside the range of representable values of type 'long unsigned int'
identify: negative or zero image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/image.c/SetImageExtent/2667.

[syominsergey]

Am I doing something wrong?
Bug is still here.
See listing.

[urban-warrior]

Thank you for reporting the issue. We have successfully reproduced it and are actively working on a patch to resolve it. You can expect this patch to be merged into the main GIT branch, later today. As part of our commitment to quality, this fix will also be included in the upcoming beta releases of ImageMagick by tomorrow. Your patience and feedback are greatly appreciated.

[syominsergey]

I'm sorry for anxiety, but problem remains in current master:

vagrant@bookworm:~/ImageMagick$ git log -1
commit 391e6db82aaf237e184569eaa1aba7cbe3c48811 (HEAD -> main, origin/main, origin/HEAD)
Author: Dirk Lemstra <dirk@lemstra.org>
Date:   Tue Apr 23 10:55:58 2024 +0200

    Update winget manifest after creating the release.
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify /home/vagrant/CVE-2023-34151/samples/bitcoin.svg
/home/vagrant/CVE-2023-34151/samples/bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.000u 0:00.009
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
MagickCore/image-private.h:139:3: runtime error: 1.84467e+19 is outside the range of representable values of type 'long unsigned int'
identify: negative or zero image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/image.c/SetImageExtent/2667.

I think, the problem with this issue is that is is in closed status.
So it is impossible to understand is the problem with mvg is really closed by last commits or not.
And it is very easy to forget about this issue with current status, as I think.
Maybe, to avoid excess notifications, it is possible to reopen this issue?
Or maybe I can create new issue for problem with mvg?
Actual status of issue will help to clear extra questions. Also it will guarantee, that the problem will not be forgotten.

[dlemstra]

This looks like an issue in your build/runtime environment @syominsergey. When stepping through this with a debugger the return value of CastDoubleToUnsigned the first time on line is MAGICK_SIZE_MAX casted to a size_t and that seems to work according to your stacktrace. Then the second time the value of x is 18446744073709551615 (MAGICK_SIZE_MAX) and that will be returned by a cast to a size_t. I don't know why this is not working in your build/run setup but we cannot reproduce your issue and that is why we will keep this issue closed.

[syominsergey]

@dlemstra, thank you for your answer!
@dlemstra, please, could you explain me - did you followed steps from my desciption posted in this issue?
I used stable debian bookworm environment from vagrant in virtual machine (virtual box) to build latest sources of ImageMagick from main branch to reproduce the problem with mvg described in this issue. I think, my environment is very usual. And it could be easily reproduced bacause of vagrant. Do you agree?

And what environment do you use?

Also I noticed, that @urban-warrior here approved that he reproduced the problem and working on it.

[syominsergey]

You can simplify my vagrant configuration like this, because you don't need, as I think, to download and import vagrant image from this page manually (I need to do this because of some reasons):

# -*- mode: ruby -*-
# vi: set ft=ruby :

Vagrant.configure("2") do |config|
  config.vm.box = "debian/bookworm64"
  config.vm.box_url = "https://app.vagrantup.com/debian/boxes/bookworm64/versions/12.20240212.1/providers/virtualbox/unknown/vagrant.box"
  config.vm.provider "virtualbox" do |v|
    v.cpus = 4
    v.memory = 4096
  end
end

All other steps, I think, will be the same. Number of cpus and memory size, as I think, are also optional and depends on resources you have on your host system.

[dlemstra]

We applied patches and can no longer reproduce this in our environment but for some reason this is still in issue on your environment. It's now up to you to debug the application and point out what kind of information we are missing and come up with a patch for this. I would advise you to add some printf statements and check if that gives you more information.

[syominsergey]

@dlemstra, thank you for your answer.
I think, I will ask for assistance debian maintainers. In all cases they need to know about last commits done in this issue to include them into patch to their package of imagemagick.

[dlemstra]

I might have solved your issue. At least that is what GitHub Copilot is thinking. This was one thing that I didn't use yet and it thinks that you are having issues with the precision limitations of a double. Can you try it again and check if that patch finally resolves the issue in your environment?

[syominsergey]

@dlemstra, sorry, what patch do you mean? Where can I get it?

[syominsergey]

I don't see any new commits, so I decided to ask where I can get patch to test.

[syominsergey]

Oh, understand, maybe you mean this commit?
148a485
I will check now.

[syominsergey]

Yes, now it looks like bug really was closed!

vagrant@bookworm:~/ImageMagick$ git log -1
commit 148a485b6bfdb557e3e40e2d04ebad9f89d01a86 (HEAD -> main, origin/main, origin/HEAD)
Author: Dirk Lemstra <dirk@lemstra.org>
Date:   Tue Apr 23 16:24:24 2024 +0200

    Use >= instead to work around precision limitations of a double.
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify /home/vagrant/CVE-2023-34151/samples/bitcoin.svg
/home/vagrant/CVE-2023-34151/samples/bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.000u 0:00.012
vagrant@bookworm:~/ImageMagick$ ./magick.sh magick identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
identify: width or height exceeds limit `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/cache.c/OpenPixelCache/3695.

@dlemstra, thank you!

[syominsergey]

Please, let me know - will all these necessary commits be backported to https://github.com/ImageMagick/ImageMagick6 ?

[syominsergey]

For now mvg bug is still in https://github.com/ImageMagick/ImageMagick6

vagrant@bookworm:~/ImageMagick6$ git log -1
commit 8f6186e4e4df5a0c18b2f64ade6cdd4f44113300 (HEAD -> main, origin/main, origin/HEAD)
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Sun Apr 21 11:13:21 2024 -0400

    beta release
vagrant@bookworm:~/ImageMagick6$ ./magick.sh identify /home/vagrant/CVE-2023-34151/samples/bitcoin.svg
/home/vagrant/CVE-2023-34151/samples/bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.000u 0:00.009
vagrant@bookworm:~/ImageMagick6$ ./magick.sh identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
coders/mvg.c:194:48: runtime error: 1.84467e+19 is outside the range of representable values of type 'long unsigned int'
identify: negative or zero image size `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/image.c/SetImageExtent/2699.

[syominsergey]

I saw some previous commits from here were backported to https://github.com/ImageMagick/ImageMagick6, so I decided to ask about possibility to backport all necessary commits into ImageMagick6 to properly close CVE-2022-32546 in that library.

[urban-warrior]

We cannot reproduce the bitcoin.svg issue you posted. We get

$ magick identify msvg:bitcoin.svg
msvg:bitcoin.svg=>bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.000u 0:00.003

Notice, no run-time exception thrown by UBSAN.

[syominsergey]

It is all ok with bitcoin.svg.
It is clear from listing.
Problem with piechart.mvg.

[syominsergey]

All recent activity here is because of piechart.mvg.

[urban-warrior]

ASAN + UBSAN enabled, from main we get:

$ magick identify mvg:piechart.mvg
identify: width or height exceeds limit `piechart.mvg' @ error/cache.c/OpenPixelCache/3695.

No undefined behaviors are thrown.

[syominsergey]

@urban-warrior, please let me know - are you checking actual main of ImageMagick6?

[syominsergey]

And please let me know: are you using all of these sanitizers as described in current issue?

./configure CFLAGS='-g -fsanitize=address,undefined,float-cast-overflow -fsanitize-recover=address,undefined,float-cast-overflow' LDFLAGS='-fsanitize=address,undefined,float-cast-overflow -lasan -lubsan'

Not only ASAN + UBSAN. I think float-cast-overflow is also important. The most important in case of current issue.

[syominsergey]

I see that now exactly after ImageMagick/ImageMagick6@be15ac9 (I checked this), the problem with mvg is also fixed for ImageMagick6:

vagrant@bookworm:~/ImageMagick6$ ./magick.sh identify /home/vagrant/CVE-2023-34151/samples/bitcoin.svg
/home/vagrant/CVE-2023-34151/samples/bitcoin.svg SVG 18446744073709551616x234 18446744073709551616x234+0+0 16-bit sRGB 19056B 0.000u 0:00.011
vagrant@bookworm:~/ImageMagick6$ ./magick.sh identify mvg:/home/vagrant/CVE-2023-34151/samples/piechart.mvg
identify: width or height exceeds limit `/home/vagrant/CVE-2023-34151/samples/piechart.mvg' @ error/cache.c/OpenPixelCache/3926.
vagrant@bookworm:~/ImageMagick6$ git log -1
commit 68162429b47c9b217bd64f9250fb38f64b050ede (HEAD -> main, origin/main, origin/HEAD)
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Sun May 12 22:15:00 2024 -0400

    add utime header

@urban-warrior, thank you!