1. 36

  2. 11

    So the quick fix is to disable http image fetching (why does ImageMagick have the ability to fetch remote images?) and a few others by adding those lines to policy.xml. Now a convert fails:

    jcs@air:~> convert "https://lobste.rs/favicon.ico" asdf.png
    convert: not authorized `//lobste.rs/favicon.ico' @ error/constitute.c/ReadImage/412.
    convert: no images defined `asdf.png' @ error/convert.c/ConvertImageCommand/3150.

    The code fix tries to sanitize the input filename, so presumably it was possible to pass a URL that was not sanitized, which caused it to execute shell commands while trying to shell out to curl. delegates.xml tells it how to shell out to these external formats, which has been modified to properly sanitize filenames.

    As far as I can tell, it’s not possible to get ImageMagick to fetch a remote URL, nor otherwise control the input filename passed to any converters in delegates.xml when you are using something like RMagick and passing in a blob of the raw image from a user, such as:

        i = Magick::ImageList.new
        i.resize_to_fit!(72, 72)
    1. 5

      Update: I was wrong, an svg blob can do remote commands.

      1. 2

        oops, duplicate post

        EDIT: lmao some of you gotta relax, I posted this before the author moved the post from Medium

        1. -7

          This doesn’t add anything to the conversation. I appreciate humor, but I would rather see intelligent discourse than pun threads and meme pics.

          1. 7

            It’s literally the homepage for the exploit.

            1. 3

              Weird - it wasn’t there when I visited it earlier. I apologize.

        2. 16

          It’s a feature!

          1. 18

            Somebody marked this incorrect, which I believe is itself incorrect. The http client code in ImageMagick didn’t write itself. It didn’t appear because somebody didn’t write it. Somebody added it, on purpose, because it’s a feature.

            I was half joking, but there is a serious point to be made here. This vulnerability exists because this feature exists.

            (By “http client” I mean “shell out to wget” or whatever, but same basic idea.)

            1. 13

              Preach! ?

              It’s sad how often the latest security-critical bug is caused by a library trying to provide clever default behaviour (so often branded as “magic”).

              I work a lot with Ruby/Rails, so naturally that’s where I’ve seen it most, but the XML/YAML RCE was a great example of this same thing.

              I’m starting to believe this category of bug is a choice. We can stop building features this way. We can live with the extra line of code whenever we want to enable an unsafe feature for trusted input.

              1. 6

                library trying to provide clever default behaviour (so often branded as “magic”)

                You do realize that “magic” is literally in the name, right?

          2. 3

            If you’re using imagemagick this century you’re very confused. It has had a consistent flow of security problems.

            1. 3

              perhaps caveat that with using it on untrusted data.

              What’s the premiere command line image smashing tool?

              1. 2

                Well, there are a great many very confused people out there. To put it mildly. And a great many people who probably don’t even know they’re very confused, since they just the image uploading feature included in their blog framework, which boils down to IM.

              2. 3

                Does this also affect GraphicsMagick?

                  1. 1

                    The instructions say to edit /etc/ImageMagick/policy.xml but on my Debian machines that file doesn’t exist (the folder does, but it’s empty). However, /etc/ImageMagick-6/policy.xml does exist and is pre-populated.

                    Is it safe to create a /etc/ImageMagick/policy.xml file and also have that /etc/ImageMagick-6/policy.xml file? Or should people choose one or the other?