From aee0940b511486b35ef2c2d0607f4cd2c0b50f23 Mon Sep 17 00:00:00 2001
From: jvoisin
Date: Fri, 22 Feb 2019 21:17:48 +0100
Subject: Mitigate filename-based race conditions

---
 main.py                 | 24 +++++++++++++++++++-----
 templates/download.html |  2 +-
 tests.py                | 15 ++++++++++-----
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/main.py b/main.py
index 82a6442..59c3791 100644
--- a/main.py
+++ b/main.py
@@ -1,4 +1,6 @@
 import os
+import hashlib
+import hmac
 
 from libmat2 import parser_factory
 
@@ -13,13 +15,20 @@ app.config['SECRET_KEY'] = os.urandom(32)
 app.config['UPLOAD_FOLDER'] = './uploads/'
 app.config['MAX_CONTENT_LENGTH'] = 16 * 1024 * 1024  # 16MB
 
-mimetypes = 'image/jpeg, image/png'
+def __hash_file(filepath: str) -> str:
+    sha256 = hashlib.sha256()
+    with open(filepath, 'rb') as f:
+        while True:
+            data = f.read(65536)  # read the file by chunk of 64k
+            if not data:
+                break
+            sha256.update(data)
+    return sha256.hexdigest()
 
 
-@app.route('/download/<string:filename>')
-def download_file(filename:str):
+@app.route('/download/<string:key>/<string:filename>')
+def download_file(key:str, filename:str):
     if filename != secure_filename(filename):
-        flash('naughty naughty')
         return redirect(url_for('upload_file'))
 
     filepath = secure_filename(filename)
@@ -27,6 +36,9 @@ def download_file(filename:str):
     complete_path = os.path.join(app.config['UPLOAD_FOLDER'], filepath)
     if not os.path.exists(complete_path):
         return redirect(url_for('upload_file'))
+    if hmac.compare_digest(__hash_file(complete_path), key) is False:
+        print('hash: %s, key: %s' % (__hash_file(complete_path), key))
+        return redirect(url_for('upload_file'))
 
     @after_this_request
     def remove_file(response):
@@ -72,7 +84,9 @@ def upload_file():
         meta_after = parser.get_meta()
         os.remove(filepath)
 
-        return render_template('download.html', mimetypes=mimetypes, meta=meta, filename=output_filename, meta_after=meta_after)
+        key = __hash_file(os.path.join(app.config['UPLOAD_FOLDER'], output_filename))
+
+        return render_template('download.html', mimetypes=mimetypes, meta=meta, filename=output_filename, meta_after=meta_after, key=key)
 
     return render_template('index.html', mimetypes=mimetypes)
 
diff --git a/templates/download.html b/templates/download.html
index cc7e150..c38b521 100644
--- a/templates/download.html
+++ b/templates/download.html
@@ -14,7 +14,7 @@ mat2 <b>could not</b> remove all the metadata from your file, those are the rema
 	</ul>
 {%endif %}
 </p>
-<a class="button button-primary" href='{{ url_for('download_file', filename=filename) }}'>⇩ Download cleaned file</a>
+<a class="button button-primary" href='{{ url_for('download_file', key=key, filename=filename) }}'>⇩ Download cleaned file</a>
 
 <hr/>
 
diff --git a/tests.py b/tests.py
index 8ce7d7e..0289755 100644
--- a/tests.py
+++ b/tests.py
@@ -25,13 +25,18 @@ class FlaskrTestCase(unittest.TestCase):
         self.assertIn(b'audio/x-flac', rv.data)
 
     def test_get_download_dangerous_file(self):
-        rv = self.app.get('/download/\..\filename')
+        rv = self.app.get('/download/1337/\..\filename')
         self.assertEqual(rv.status_code, 302)
 
-    def test_get_download_nonexistant_file(self):
+    def test_get_download_without_key_file(self):
         rv = self.app.get('/download/non_existant')
+        self.assertEqual(rv.status_code, 404)
+
+    def test_get_download_nonexistant_file(self):
+        rv = self.app.get('/download/1337/non_existant')
         self.assertEqual(rv.status_code, 302)
 
+
     def test_get_upload_without_file(self):
         rv = self.app.post('/')
         self.assertEqual(rv.status_code, 302)
@@ -66,13 +71,13 @@ class FlaskrTestCase(unittest.TestCase):
                 data=dict(
                     file=(io.BytesIO(b"Some text"), 'test.txt'),
                     ), follow_redirects=True)
-        self.assertIn(b'/download/test.cleaned.txt', rv.data)
+        self.assertIn(b'/download/4c2e9e6da31a64c70623619c449a040968cdbea85945bf384fa30ed2d5d24fa3/test.cleaned.txt', rv.data)
         self.assertEqual(rv.status_code, 200)
 
-        rv = self.app.get('/download/test.cleaned.txt')
+        rv = self.app.get('/download/4c2e9e6da31a64c70623619c449a040968cdbea85945bf384fa30ed2d5d24fa3/test.cleaned.txt')
         self.assertEqual(rv.status_code, 200)
 
-        rv = self.app.get('/download/test.cleaned.txt')
+        rv = self.app.get('/download/4c2e9e6da31a64c70623619c449a040968cdbea85945bf384fa30ed2d5d24fa3/test.cleaned.txt')
         self.assertEqual(rv.status_code, 302)
 
 
-- 
cgit v1.3