Prevent exiftool-based parameter-injection

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)

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')