Merge commit from fork

* Security: prevent XSS with storage path template rendering

* Security: prevent XSS svg uploads

* Security: force attachment disposition for logo

* Add suggestions from code review

* Improve SVG validation with allowlist for tags and attributes
This commit is contained in:
shamoon
2025-08-16 07:34:00 -07:00
committed by GitHub
parent 42bdbc1b2d
commit b1c406680f
9 changed files with 179 additions and 10 deletions

View File

@@ -0,0 +1,4 @@
<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">
<text x="10" y="20">Hello</text>
<script>alert('XSS')</script>
</svg>

After

Width:  |  Height:  |  Size: 140 B

View File

@@ -149,6 +149,11 @@ class TestApiAppConfig(DirectoriesMixin, APITestCase):
THEN:
- old app_logo file is deleted
"""
admin = User.objects.create_superuser(username="admin")
self.client.force_login(user=admin)
response = self.client.get("/logo/")
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
with (Path(__file__).parent / "samples" / "simple.jpg").open("rb") as f:
self.client.patch(
f"{self.ENDPOINT}1/",
@@ -156,6 +161,12 @@ class TestApiAppConfig(DirectoriesMixin, APITestCase):
"app_logo": f,
},
)
# Logo exists at /logo/simple.jpg
response = self.client.get("/logo/simple.jpg")
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertIn("image/jpeg", response["Content-Type"])
config = ApplicationConfiguration.objects.first()
old_logo = config.app_logo
self.assertTrue(Path(old_logo.path).exists())
@@ -168,6 +179,26 @@ class TestApiAppConfig(DirectoriesMixin, APITestCase):
)
self.assertFalse(Path(old_logo.path).exists())
def test_api_rejects_malicious_svg_logo(self):
"""
GIVEN:
- An SVG logo containing a <script> tag
WHEN:
- Uploaded via PATCH to app config
THEN:
- SVG is rejected with 400
"""
path = Path(__file__).parent / "samples" / "malicious.svg"
with path.open("rb") as f:
response = self.client.patch(
f"{self.ENDPOINT}1/",
{"app_logo": f},
format="multipart",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn("disallowed", str(response.data).lower())
def test_create_not_allowed(self):
"""
GIVEN: