Commit aee0940b authored by jvoisin's avatar jvoisin

Mitigate filename-based race conditions

parent 12be5359
Pipeline #22905 passed with stages
in 1 minute and 7 seconds
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)
......
......@@ -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/>
......
......@@ -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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment