Skip to content

Use strict filetype checks when calling `gm`

Matthias Käppler requested to merge mk/strict-filetype-check into master

Overview

This is an experimental, unreleased feature.

By default, gm will ignore file extensions and determine the correct decoder/encoder based on the file's magic bytes. This means that a file pretending to be something else based on its extension might get processed.

According to http://www.graphicsmagick.org/security.html it is recommended to specify the reader based on the expected type by prefixing the input file with jpg: or png: respectively.

In gitlab!39965 (merged) we now send the image MIME type in the send-data header, based on the file's extension i.e. what it claims to be. If gm finds that the expected format does not match the actual format of the file, it will bail out with an error now. Unfortunately I did not find a good way to fail-over to serving the original image in this case, because by the time we get an error from gm we are already writing to the response writer. So for now I decided to fail fast with a 500.

I also started to add unit tests for the imageresizer module, not all of which are related to these specific changes.

Behavior

I prepared a "fake" JPG file avatar_w300_fake.jpg, which carries the .jpg extension but is actually a PNG file.

GET http://localhost:3000/uploads/-/system/group/avatar/22/avatar_w300_fake.jpg?width=64

Before

workhorse_1      | time="2020-08-21T13:33:28Z" level=info msg="ImageResizer: success" bytes_written=10408 correlation_id=qsp3RHnZA44
workhorse_1      | localhost:3000 172.17.0.1 - - [2020/08/21:13:33:28 +0000] "GET /uploads/-/system/group/avatar/22/avatar_w300_fake.jpg?width=64 HTTP/1.1" 200 10408 "http://localhost:3000/gitlab-org" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.135 Safari/537.36" 168

After

workhorse_1      | time="2020-08-21T14:06:47Z" level=info msg="gm convert: Not a JPEG file: starts with 0x89 0x50 (-) [No such file or directory]." correlation_id=QTz6l24lvS4
workhorse_1      | time="2020-08-21T14:06:47Z" level=error msg=error correlation_id=QTz6l24lvS4 error="ImageResizer: failed writing output stream" method=GET uri="/uploads/-/system/group/avatar/22/avatar_w300_fake.jpg?width=64"
Edited by Nick Thomas

Merge request reports