ImageMagick tries to resize from url

[thperret]

Hi,

When trying to access an image uploaded through GrapeJS editor, I get the following error:

Command failed: identify: not authorized '//example.org/grapejs/uploads/0/logo-.png' @ error/constitute.c/ReadImage/454.

(domain name was replaced by example.org).

The image (from the HTML of the mail) url is: https://example.org/editorapi/img?src=https%3A%2F%2Fexample.org%2Fgrapejs%2Fuploads%2F0%2Flogo.png&method=resize&params=129%2C71.

It seems to comes from the gm package which try to resize the image using the direct url (not the path of the image file).

I tried directly the command: convert http://i.imgur.com/nTheJ.jpg -resize 600x600 /tmp/test.jpg which gives a similar issue:

convert: not authorized `//i.imgur.com/nTheJ.jpg' @ error/constitute.c/ReadImage/454.
convert: no images defined `/tmp/test.jpg' @ error/convert.c/ConvertImageCommand/3046.

Thanks

[witzig]

Looks like your ImageMagick Security Policy doesn't grant any rights for opening URLs, i.e. pattern HTTPS. I think policy.xml can be found at /etc/ImageMagick-6/policy.xml, could you check this?

Also, I assume you didn't modify the policy.xml but it's the default for your OS?

[thperret]

I'm using a docker image (built from the Dockerfile available in this repo). The image is based on CentOs and I didn't changed the default ImageMagick policy. Indeed, it looks like the policy prevents opening URLs:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE policymap [
<!ELEMENT policymap (policy)+>
<!ELEMENT policy (#PCDATA)>
<!ATTLIST policy domain (delegate|coder|filter|path|resource) #IMPLIED>
<!ATTLIST policy name CDATA #IMPLIED>
<!ATTLIST policy rights CDATA #IMPLIED>
<!ATTLIST policy pattern CDATA #IMPLIED>
<!ATTLIST policy value CDATA #IMPLIED>
]>
<!--
  Configure ImageMagick policies.

  Domains include system, delegate, coder, filter, path, or resource.

  Rights include none, read, write, and execute.  Use | to combine them,
  for example: "read | write" to permit read from, or write to, a path.

  Use a glob expression as a pattern.

  Suppose we do not want users to process MPEG video images:

    <policy domain="delegate" rights="none" pattern="mpeg:decode" />

  Here we do not want users reading images from HTTP:

    <policy domain="coder" rights="none" pattern="HTTP" />

  Lets prevent users from executing any image filters:

    <policy domain="filter" rights="none" pattern="*" />

  The /repository file system is restricted to read only.  We use a glob
  expression to match all paths that start with /repository:

    <policy domain="path" rights="read" pattern="/repository/*" />

  Let's prevent possible exploits by removing the right to use indirect reads.

    <policy domain="path" rights="none" pattern="@*" /> 

  Any large image is cached to disk rather than memory:

    <policy domain="resource" name="area" value="1GB"/>

  Define arguments for the memory, map, area, and disk resources with
  SI prefixes (.e.g 100MB).  In addition, resource policies are maximums for
  each instance of ImageMagick (e.g. policy memory limit 1GB, -limit 2GB
  exceeds policy maximum so memory limit is 1GB).
-->
<policymap>
  <!-- <policy domain="system" name="precision" value="6"/> -->
  <!-- <policy domain="resource" name="temporary-path" value="/tmp"/> -->
  <!-- <policy domain="resource" name="memory" value="2GiB"/> -->
  <!-- <policy domain="resource" name="map" value="4GiB"/> -->
  <!-- <policy domain="resource" name="area" value="1GB"/> -->
  <!-- <policy domain="resource" name="disk" value="16EB"/> -->
  <!-- <policy domain="resource" name="file" value="768"/> -->
  <!-- <policy domain="resource" name="thread" value="4"/> -->
  <!-- <policy domain="resource" name="throttle" value="0"/> -->
  <!-- <policy domain="resource" name="time" value="3600"/> -->
  <policy domain="coder" rights="none" pattern="EPHEMERAL" />
  <policy domain="coder" rights="none" pattern="HTTPS" />
  <policy domain="coder" rights="none" pattern="HTTP" />
  <policy domain="coder" rights="none" pattern="URL" />
  <policy domain="coder" rights="none" pattern="FTP" />
  <policy domain="coder" rights="none" pattern="MVG" />
  <policy domain="coder" rights="none" pattern="MSL" />
  <policy domain="coder" rights="none" pattern="TEXT" />
  <policy domain="coder" rights="none" pattern="LABEL" />
  <policy domain="path" rights="none" pattern="@*" />
</policymap>

After commenting out the <policy domain="coder" rights="none" pattern="HTTP" /> the error is gone!

Thanks a lot for your fast reply and fix! Now, I need to find how to restrict it to my domain!

I think it would be great to add an entry in the wiki or the readme for this issue (with of course some warnings about possible flaws).

[witzig]

The image is based on CentOs and I didn't changed the default ImageMagick policy.

Thanks for the info. In this case we should change the handling in routes/editorapi.js and not rely on ImageMagick being able to open URLs.

I'm working on this.

[witzig]

@thperret, the image handling has been changed with the latest commit, so this should be fixed now. Thanks again for the detailed report and sorry for inconvenience. :-/

[thperret]

@witzig, no problem! And thanks a lot for the very fast workaround and fix!

As a side note: do you have an (approximate) ETA for the next release that will include the fix? For now I would prefer to build a docker image from a tag release. Thanks again

[witzig]

As a side note: do you have an (approximate) ETA for the next release that will include the fix?

@andris9, should we prepare a "last last" release with works on Node 6? (The async/await stuff is currently only used by e2e tests.) The following has been fixed since v1.24.0:

• Throttling and pausing not working as expected
• HTML templates for custom JSON fields not rendering properly (escaped HTML tags)
• Merge Tags for non custom text fields couldn't be merged
• Blacklisted emails counter not set to 0 on campaign reset
• Hidden custom fields (for custom forms) losing values when updating preferences

For a v2 release as discussed here we should wait for geoip-ultralight to bump some dependencies first.