This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/allura.git
commit 4f58190ac1d5740a21dda9fddb4d735257563568 Author: Dave Brondsema <[email protected]> AuthorDate: Tue May 19 12:36:38 2026 -0400 [#8607] harden attachment upload --- Allura/allura/app.py | 2 +- Allura/allura/lib/helpers.py | 10 ++++++++++ Allura/allura/lib/utils.py | 2 +- Allura/allura/model/attachments.py | 2 +- Allura/allura/model/discuss.py | 2 +- Allura/allura/tests/model/test_filesystem.py | 23 +++++++++++++++++++++++ ForgeFiles/forgefiles/model/files.py | 2 +- ForgeTracker/forgetracker/model/ticket.py | 2 +- ForgeWiki/forgewiki/model/wiki.py | 2 +- 9 files changed, 40 insertions(+), 7 deletions(-) diff --git a/Allura/allura/app.py b/Allura/allura/app.py index 78798f7f5..22762f33e 100644 --- a/Allura/allura/app.py +++ b/Allura/allura/app.py @@ -808,7 +808,7 @@ def save_attachments(self, path, attachments): for attachment in attachments: attachment_path = os.path.join( path, - os.path.basename(attachment.filename) + h.safe_filename(attachment.filename) ) with open(attachment_path.encode('utf8', 'replace'), 'wb') as fl: fl.write(attachment.rfile().read()) diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py index 33d180790..07204583f 100644 --- a/Allura/allura/lib/helpers.py +++ b/Allura/allura/lib/helpers.py @@ -272,6 +272,16 @@ def encodings(): return _attempt_encodings(s, encodings()) +def safe_filename(filename, fallback='unnamed'): + # strip slashes and backslashes and null bytes for good measure + # (os.path.basename on Linux treats backslashes as regular characters) + filename = really_unicode(filename).replace('\\', '/').replace('\x00', '') + filename = os.path.basename(filename) + if filename in ('', '.', '..'): + filename = fallback + return filename + + def find_user(email): from allura import model as M return M.User.by_email_address(email) diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py index a7b416be8..7dd73f4ed 100644 --- a/Allura/allura/lib/utils.py +++ b/Allura/allura/lib/utils.py @@ -506,7 +506,7 @@ def serve_file(fp, filename, content_type, last_modified=None, if etag: etag_cache(etag) tg.response.headers['Content-Type'] = '' - tg.response.content_type = str(content_type) + tg.response.content_type = content_type.split('\n', 1)[0].split('\r', 1)[0] # enforce single line, no header splitting tg.response.cache_expires = cache_expires or asint( tg.config.get('files_expires_header_secs', 60 * 60)) tg.response.last_modified = last_modified diff --git a/Allura/allura/model/attachments.py b/Allura/allura/model/attachments.py index cc91468f6..06d6bcaf6 100644 --- a/Allura/allura/model/attachments.py +++ b/Allura/allura/model/attachments.py @@ -67,7 +67,7 @@ def metadata_for(cls, artifact): @classmethod def save_attachment(cls, filename, fp, content_type=None, **kwargs): - filename = h.really_unicode(filename) + filename = h.safe_filename(filename) thumbnail_meta = dict(type="thumbnail", app_config_id=c.app.config._id) thumbnail_meta.update(kwargs) original_meta = dict(type="attachment", app_config_id=c.app.config._id) diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py index 3189db324..579a32aa6 100644 --- a/Allura/allura/model/discuss.py +++ b/Allura/allura/model/discuss.py @@ -204,7 +204,7 @@ def attachment_for_export(self, page): str(self.artifact._id), self._id, page.slug, - os.path.basename(attach.filename)) + h.safe_filename(attach.filename)) ) for attach in page.attachments] def attachments_for_json(self, page): diff --git a/Allura/allura/tests/model/test_filesystem.py b/Allura/allura/tests/model/test_filesystem.py index 844a7b7b2..802ad8ffb 100644 --- a/Allura/allura/tests/model/test_filesystem.py +++ b/Allura/allura/tests/model/test_filesystem.py @@ -214,6 +214,29 @@ def test_attachment_name_encoding(self): assert not isinstance(attachment, tuple) # tuple is for (img, thumb) pairs assert attachment.filename == 'Strukturpr\xfcfung.dvi' + def test_attachment_filename_path_traversal_sanitization(self): + c.app.config._id = None + attachment = M.BaseAttachment.save_attachment( + '../../../etc/passwd', BytesIO(b'evil data'), + save_original=True) + assert attachment.filename == 'passwd' + + def test_attachment_filename_windows_path_sanitization(self): + # backslashes are not separators on Linux but are on Windows, so a zip + # entry like '..\\..\\evil.exe' would zip-slip on Windows extraction + c.app.config._id = None + attachment = M.BaseAttachment.save_attachment( + '..\\..\\..\\Windows\\evil.exe', BytesIO(b'evil data'), + save_original=True) + assert attachment.filename == 'evil.exe' + + def test_attachment_filename_dot_sanitization(self): + c.app.config._id = None + for bad in ('..', '.', '', 'foo/', 'bar\\'): + attachment = M.BaseAttachment.save_attachment( + bad, BytesIO(b'data'), save_original=True) + assert attachment.filename == 'unnamed' + def _assert_content(self, f, content): result = f.rfile().read() assert result == content, result diff --git a/ForgeFiles/forgefiles/model/files.py b/ForgeFiles/forgefiles/model/files.py index ee421a4b9..7d285aaa1 100755 --- a/ForgeFiles/forgefiles/model/files.py +++ b/ForgeFiles/forgefiles/model/files.py @@ -213,7 +213,7 @@ def readme(self): @classmethod def save_attachment(cls, filename, fp, content_type=None, **kwargs): - filename = h.really_unicode(filename) + filename = h.safe_filename(filename) original_meta = dict(type="project_file", app_config_id=c.app.config._id, project_id=c.project._id) original_meta.update(kwargs) fp.seek(0) diff --git a/ForgeTracker/forgetracker/model/ticket.py b/ForgeTracker/forgetracker/model/ticket.py index f95419a0f..c9a61cccb 100644 --- a/ForgeTracker/forgetracker/model/ticket.py +++ b/ForgeTracker/forgetracker/model/ticket.py @@ -1189,7 +1189,7 @@ def attachments_for_export(self): path=os.path.join( self.app_config.options.mount_point, str(self._id), - os.path.basename(attach.filename))) for attach in self.attachments] + h.safe_filename(attach.filename))) for attach in self.attachments] def attachments_for_json(self): return [dict(bytes=attach.length, diff --git a/ForgeWiki/forgewiki/model/wiki.py b/ForgeWiki/forgewiki/model/wiki.py index 712857eaf..91da0ca5e 100644 --- a/ForgeWiki/forgewiki/model/wiki.py +++ b/ForgeWiki/forgewiki/model/wiki.py @@ -145,7 +145,7 @@ def attachments_for_export(self): path=os.path.join( self.app_config.options.mount_point, str(self._id), - os.path.basename(attach.filename))) for attach in self.attachments] + h.safe_filename(attach.filename))) for attach in self.attachments] def attachments_for_json(self): return [dict(bytes=attach.length,
