From 6a832a4104f2da3985f605e9bb973591f19d30f7 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 6 Jun 2018 23:50:25 +0200 Subject: [PATCH] Prevent exiftool-based parameter-injection --- libmat2/images.py | 39 +++++++++++++++++++++++---------------- tests/test_libmat2.py | 10 ++++++++++ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/libmat2/images.py b/libmat2/images.py index c84952a..e425735 100644 --- a/libmat2/images.py +++ b/libmat2/images.py @@ -1,6 +1,8 @@ import subprocess import json import os +import shutil +import tempfile import cairo @@ -11,7 +13,26 @@ from gi.repository import GdkPixbuf from . import abstract -class PNGParser(abstract.AbstractParser): +class __ImageParser(abstract.AbstractParser): + def get_meta(self): + """ There is no way to escape the leading(s) dash(es) of the current + self.filename to prevent parameter injections, so we do have to copy it + """ + fname = self.filename + tmpdirname = "" + if self.filename.startswith('-'): + tmpdirname = tempfile.mkdtemp() + fname = os.path.join(tmpdirname, self.filename) + shutil.copy(self.filename, fname) + out = subprocess.check_output(['/usr/bin/exiftool', '-json', fname]) + if self.filename.startswith('-'): + shutil.rmtree(tmpdirname) + meta = json.loads(out.decode('utf-8'))[0] + for key in self.meta_whitelist: + meta.pop(key, None) + return meta + +class PNGParser(__ImageParser): mimetypes = {'image/png', } meta_whitelist = {'SourceFile', 'ExifToolVersion', 'FileName', 'Directory', 'FileSize', 'FileModifyDate', @@ -28,30 +49,16 @@ class PNGParser(abstract.AbstractParser): except MemoryError: raise ValueError - def get_meta(self): - out = subprocess.check_output(['/usr/bin/exiftool', '-json', self.filename]) - meta = json.loads(out.decode('utf-8'))[0] - for key in self.meta_whitelist: - meta.pop(key, None) - return meta - def remove_all(self): surface = cairo.ImageSurface.create_from_png(self.filename) surface.write_to_png(self.output_filename) return True -class GdkPixbufAbstractParser(abstract.AbstractParser): +class GdkPixbufAbstractParser(__ImageParser): """ GdkPixbuf can handle a lot of surfaces, so we're rending images on it, this has the side-effect of removing metadata completely. """ - def get_meta(self): - out = subprocess.check_output(['/usr/bin/exiftool', '-json', self.filename]) - meta = json.loads(out.decode('utf-8'))[0] - for key in self.meta_whitelist: - meta.pop(key, None) - return meta - def remove_all(self): _, extension = os.path.splitext(self.filename) pixbuf = GdkPixbuf.Pixbuf.new_from_file(self.filename) diff --git a/tests/test_libmat2.py b/tests/test_libmat2.py index 89a5811..84eb75e 100644 --- a/tests/test_libmat2.py +++ b/tests/test_libmat2.py @@ -17,6 +17,16 @@ class TestParserFactory(unittest.TestCase): self.assertEqual(parser.__class__, audio.MP3Parser) +class TestParameterInjection(unittest.TestCase): + def test_ver_injection(self): + shutil.copy('./tests/data/dirty.png', './-ver') + p = images.PNGParser('-ver') + meta = p.get_meta() + self.assertEqual(meta['Comment'], 'This is a comment, be careful!') + self.assertEqual(meta['ModifyDate'], "2018:03:20 21:59:25") + os.remove('-ver') + + class TestUnsupportedFiles(unittest.TestCase): def test_pdf(self): shutil.copy('./tests/test_libmat2.py', './tests/clean.py')