(always) be careful with image EXIF-data

1

First of all, I reported this “issue” to Kirby, but the moderators (with outstanding good support) decided it was a real Kirby-threat, because it’s not 100% Kirby-only-related (and not an issue when you’re not explicitly doing the wrong things).

Issues with EXIF (meta) data in images are a big thing these days

According to the first article (in the list above) Wordpress is vulnerable for EXIF-data, because that data can contain (javascript) code. So when you query information about a camera-model, the ISO or used white-balance, some embedded code (inside the same EXIF container) can also be parsed.

Normally the EXIF data looks something like this;

Camera  : Lumia 950XL
Author  : Bill Gates
ISO     : 3200
Flash   : no
Caption : Nice shot of fireworks by daylight

But everyone is free to add data like this;

Camera  : Lumia 950XL
Author  : Bill Gates
ISO     : 3200
Flash   : no
Caption : Nice shot of fireworks by daylight<script>var iframe = document.createElement('iframe');iframe.src = 'http://www.bild.de/';document.body.appendChild(iframe);</script>

You see what I did there? I extended the description tag with a nifty javascript (which injects an iFrame after the description itself).

###So what’s all the fuzz about?

Well, image this scenario;

  1. I create a nice image, let’s say a Kirby logo
  2. Inside the EXIF data I embed a nice caption; Kirby is so cool!
  3. But, wait! …I also inject the javascript…

Pasted image941×733 53.5 KB

On the “outersight” you will see nothing about the inject script (you can see it in the screenshot above).

The image just looks like… an image;

But try to download the (original) image and see for your self;

http://www.fotothing.com/photos/fb3/fb3e0755b710f2ae98d4d0c084aa49eb.jpg

(I placed it on that site, because it doesn’t strip EXIF while uploading).

###On second sight

When you open this image inside an image-editor (that supports EXIF) or just a plain text reader you will notice some…

Pasted image1246×730 28 KB

The meta-data does not only contains the “right” text Kirby is so Cool! but also the script after it.

Now let’s continue our scenario;

  1. You like the image and decides to place it on your site.
  2. You also want to show it’s title / caption, so you create a Kirby-query like this;
  echo '<img src="' . $page->file($page->img())->url() . '"><br>';
  $image = $page->file($page->img());
  $exif = $image->exif();
  echo $exif->data()['ImageDescription'];

The script above doesn’t do much;

  • It writes down an image (with the fieldname which is called img().
  • It queries it’s exif-data.
  • It parses the description and writes it down.

###So what?

…well… here we are;

It parses the description and writes it down.

And we all know what the description is; a nifty javascript that injects an iFrame…

The screenshot above is a real-life implementation of both the (infected) image and the php script.

Querying the EXIF-data not only writes down the description of the image, but also the script. In this case the iFrame is visible and harmless, but of course one can make it invisible and harmful within the same line of code.

###Now what?

Well, one thing; as always be sure to check your images, the assets and don’t write out code without escaping any tags.

The right output of the meta-data should be something like;

&lt;script&gt;var iframe = document.createElement('iframe');iframe.src = 'http://www.bild.de/';document.body.appendChild(iframe);&lt;/script&gt;

So, not a fully functional, executable script-tag.

Please mention, the Kirby moderators are aware of the issue, they supported my report withing 1 hour and this issue is not only Kirby related;

It’s just good practice to never trust user-input or third-party-input… check your assets!

2 Likes
2

Thank you for posting this.

Just to make it clear: We do actually think it is a very important issue, but as you said, Kirby is not directly affected by it. It’s just a security issue that users of any CMS can get into if they don’t do it right.

BTW, the code that does the transform from the “bad code” to safe text is as simple as the following line:

echo esc($exif->data()['ImageDescription']);
3 Likes
3

Well, you wrote down the solution in one line of code :slight_smile:

Thanks for that - I just tried to “warn / inform” other users (not only Kirby users, but all users) to be careful when “parsing” data which is not their own.

You can simply download an image, “spit” out it’s values (without checking them first) and create a situation described above.

Of course this is an extreme small change (and not Kirby related), it’s just “good” practice :slight_smile:

1 Like
4

Yep. There are so many ways of how you can get into XSS situations. Always be careful.

5

In one line, that's what I wanted to say .... I must learn to keep things simple!

6

No. I think you did a really good job explaining the problem in detail. Even inexperienced developer could understand what’s the problem. Not everybody knows what „prevent XSS by using esc()“ means :wink:

2 Likes
7

Thanks - that was, indeed, the intention of my post. :slight_smile:

8

Thanks for sharing this. It’s good to know. Having a file upload hook that replaces the exit data with escaped versions may be a great security feature.

1 Like
9

Or maybe Kirby must escape the meta-data by default.

And if you want the un-escaped data, you must declare this explicit :slight_smile:

1 Like
10

Yeah exactly! That’s what I was trying to say in my message above, it could be integrated in the core ! And the option to prevent the esc is great thinking too!

11

So Kirby must - by default - be secure / escape everything (most of the
times, that’s what you want).

And if you want it “unsecure” just add it to the parameter of your
php-string :slight_smile:

1 Like
12

After you told us about the issue, I wrote that escaping always needs to be context-sensitive.

Replacing special HTML characters with entities works for HTML, but not for JSON, YAML, JavaScript or any other language. That’s why escaping is unfortunately always the responsibility of the developer who writes the code that transforms data into a different format. In case of templates, that’s the Kirby user.

While I think that your idea is great, we unfortunately can’t do this directly in Kirby.

1 Like
13

BTW: If you happen to have users who are prone to enter malicious content into the panel, you have the same problem. If you can’t trust them, escape your output in your templates.

14

You’re right - but I did not even think of “users”, but site-owners who upload a favorite (malicious) image - without checking the meta-data.

Of course this is not a real live scenario, but it can happen (create a populair image of a very rare Pokémon animal, populate it with malicious meta-data, be sure to have your image uploaded by tons of un-aware webmasters).

When someone does query the malicious meta-data of that very populair Pokémon-animal (without checking it), the affected script can become active.

Again; it’s not a real live scenario - but I think consciousness about the possibility is never a bad thing :slight_smile: