How to prevent leakage of exif information?

Kirby 3
1

Hey there!

Most photos contain sensitive exif data, like geo coordinates. As I just found out via a blogpost by Chance Arthur, Kirby does not keep uploaded files private and original file sizes, together with sensitive information like EXIF, always are freely accessible.

Is Chance Arthurs plugin the proper way to solve this issue or are there even better alternatives? Is the security aspect possibly important enough that perhaps a Kirby core functionality should be implemented?

Thanks!

1 Like
2

I think the plugins approach is valid: writing 2 hooks that automatically treat your files when they’re uploaded. That is opinionated enough to not be in the core though (you wanna resize too? If yes how much? etc).

For a feature request I’d rather see the ability to extend the FileRules. Not all files are automatically published, a way to add filters to the already present ones could be helpful.

Something like (this is a proposal, doesn’t work):

Kirby::plugin('rasteiner/privatefiles', [
    'validFile' => function($file) {
        //publish file only if its not a large image
        return !$file->isResizable() || max($file->width(), $file->height()) < 500;
    }
]);
3

Provided that the content folder is not accessible (like it should be) and unless I’m missing something, this should also be achievable via a file::url() component in combination with a route instead of the hooks. Which would also preserve the original files in the content folder in case the files with this information are needed for any purpose (only make files with exif data accessible to buyers, clients etc):

<?php

use Kirby\Cms\App;

App::plugin('texnixe/file-resizer', [

    'components' => [
        'file::url' => function ($kirby, $file) {
            if ($file->type() === 'image') {
              return $kirby->url() . '/' . $file->parent()->id() . '/' . $file->filename();
            }
            return $file->mediaUrl();
          },

    ],
    'routes' => [
        [
            'pattern' => '(:all)/(:any)',
            'action' => function($page, $file) {
                if (($page = page($page)) && $file = $page->file($file)) {
                    return $file->resize(1800)->read();
                }
            }
        ]
    ]

]);
4

Hmm… I just tried that file::url component and while this indeed works for page($pageid)->file($filename)->url(), a call for a thumb URL like page($pageid)->file($filename)->thumb(['width' => 800])->url() still returns the URL to the media folder. And once I have the media URL to a thumb, I can strip the -800x part and Kirby automatically makes the original file available in the media folder :thinking:

I’d somehow wish there were a way to prevent Kirby from generating a copy of the original file in the media folder, while being able to continue using the thumbs engine etc. Is there a way to tap into that validFile method @rasteiner pointed at? Or any other place where I could use a plugin to tell Kirby “don’t create 1:1 copies of image files in the media folder” while all the other core functionalities for those files, such as the thumb engine, remain unchanged?

1 Like
5

This brain teaser of “how to keep certain files out of the media folder” kept hovering in my head. I did a bit of digging in the core. To test, I modified the FileActions::publish() method (other possibilities would be Media::publish or FileRules::validFile(), but I’m unsure whether these are also relevant in other flows; FileActions::publish() seems most specific for the task at hand?):

public function publish()
{
    // experimental addition starts
    if($this->type() == 'image' && $this->mime() == 'image/jpeg') {
        go('error');
    }
    // experimental addition ends
    Media::publish($this, $this->mediaRoot());
    return $this;
}

Thumbs still work, but the original file for JPGs of type image are no longer copied into the media folder. Instead of redirecting to the error page (I couldn’t figure out how to throw a real 404 at this point), one could probably call a custom publish method here – e.g. to create a EXIF-free version, if desired.

Would this be possible to be turned into a plugin by replacing the core method? But the maybe more important question is: what other functionalities would this break? (Disclaimer: This is really just some explorative hacking; don’t use in production…)

6

The team could think about allowing “file models”, so people could override the publish method.

Or currently we could think of returning a custom “Files” collection from certain page models. Like (pseudocode):

class PrivateFile extends Kirby\Cms\File {
    public function publish() {
        throw new Exception('nope!');
    }
}

class PrivateFiles extends Kirby\Cms\Files {
    public function factory(array $data) {
        //build a collection of PrivateFile
    }
}

class GalleryStorePage extends Kirby\Cms\Page {
    public function files() {
        return PrivateFiles::factory($this->inventory(... $etc));
    }
}

My reasoning was that for this usecase we actually don’t want to be able to specify “what happens when we publish”, but more “what files should be publishable”.
But “what happens when we publish” could also be very interesting for other use cases: like when I want to publish on a cdn, for example.

1 Like
7

That’s what’s definitely missing, you currently would’ve to go a long way for that, or would have to limit it to particular pages.

1 Like
8

why not create a thumb of the original on upload, striping all meta data with a custom thumbdriver but copy the sensitive data into the content file?

(edited: oh. thats what arthurs plugin seems to do)

9

That is exactly what the plugin mentioned in the first post does. But it would be great if there was some other way of preserving the original files in the content folder, i.e. prevent publishing of them.

1 Like