You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2026/05/26 20:56:34 UTC
(allura) 18/33: [#8607] harden attachment upload
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch db/8607
in repository https://gitbox.apache.org/repos/asf/allura.git
commit 273ef2ff5b5b31957577a2a9a561046476224968
Author: Dave Brondsema <da...@brondsema.net>
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 6c31a8abc..028c2cb43 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 bbec87215..a2e51a334 100644
--- a/ForgeTracker/forgetracker/model/ticket.py
+++ b/ForgeTracker/forgetracker/model/ticket.py
@@ -1187,7 +1187,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,