Skip to content

Commit ee7481b

Browse files
JacobCoffeeclaude
andauthored
Fix CodeQL security alerts (#2927)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 55a0162 commit ee7481b

File tree

10 files changed

+61
-45
lines changed

10 files changed

+61
-45
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ name: CI
22

33
on: [push, pull_request, workflow_dispatch]
44

5+
permissions: {}
6+
57
jobs:
68
migrations:
79
if: github.event_name != 'push' || github.event.repository.fork == true || github.ref == 'refs/heads/main'

.github/workflows/static.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ name: Check collectstatic
22

33
on: [push, pull_request, workflow_dispatch]
44

5+
permissions: {}
6+
57
jobs:
68
collectstatic:
79
if: github.event_name != 'push' || github.event.repository.fork == true || github.ref == 'refs/heads/main'

apps/events/templates/events/event_detail.html

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,10 @@ <h3 class="widget-title">More events in <!-- The current category--><a href="{{
184184
<script>
185185
$(function(){
186186
$('#dataRangeSelect').change(function(e) {
187-
window.location = $(this).val();
187+
let url = $(this).val();
188+
if (url && url.charAt(0) === '/' && url.charAt(1) !== '/') {
189+
window.location = url;
190+
}
188191
});
189192
});
190193
</script>

apps/mailing/forms.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Forms for the mailing app."""
22

33
from django import forms
4-
from django.template import Context, Template, TemplateSyntaxError
4+
from django.template import Context, TemplateSyntaxError
55

66
from apps.mailing.models import BaseEmailTemplate
77

@@ -13,7 +13,7 @@ def clean_content(self):
1313
"""Validate that the content field contains valid Django template syntax."""
1414
content = self.cleaned_data["content"]
1515
try:
16-
template = Template(content)
16+
template = BaseEmailTemplate.template_engine.from_string(content)
1717
template.render(Context({}))
1818
except TemplateSyntaxError as e:
1919
raise forms.ValidationError(e) from e

apps/mailing/models.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from django.core.mail import EmailMessage
44
from django.db import models
5-
from django.template import Context, Template
5+
from django.template import Context, Engine
66
from django.urls import reverse
77

88

@@ -33,15 +33,20 @@ def preview_content_url(self):
3333
url_name = f"admin:{prefix}_preview"
3434
return reverse(url_name, args=[self.pk])
3535

36+
template_engine = Engine(
37+
builtins=["django.template.defaultfilters"],
38+
autoescape=True,
39+
)
40+
3641
def render_content(self, context):
37-
"""Render the email body using the Django template engine."""
38-
template = Template(self.content)
42+
"""Render the email body using a sandboxed Django template engine."""
43+
template = self.template_engine.from_string(self.content)
3944
ctx = Context(context)
4045
return template.render(ctx)
4146

4247
def render_subject(self, context):
43-
"""Render the email subject using the Django template engine."""
44-
template = Template(self.subject)
48+
"""Render the email subject using a sandboxed Django template engine."""
49+
template = self.template_engine.from_string(self.subject)
4550
ctx = Context(context)
4651
return template.render(ctx)
4752

apps/pages/management/commands/fix_success_story_images.py

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import re
2-
from pathlib import Path
2+
from pathlib import PurePosixPath
33
from urllib.parse import urlparse
44

55
import requests
6-
from django.conf import settings
7-
from django.core.files import File
6+
from django.core.files.base import ContentFile
87
from django.core.management.base import BaseCommand
98

10-
from apps.pages.models import Image, Page, page_image_path
9+
from apps.pages.models import Image, Page
1110

1211
HTTP_OK = 200
1312

@@ -18,14 +17,6 @@ class Command(BaseCommand):
1817
def get_success_pages(self):
1918
return Page.objects.filter(path__startswith="about/success/")
2019

21-
def image_url(self, path):
22-
"""
23-
Given a full filesystem path to an image, return the proper media
24-
url for it
25-
"""
26-
new_url = path.replace(settings.MEDIA_ROOT, settings.MEDIA_URL)
27-
return new_url.replace("//", "/")
28-
2920
def fix_image(self, path, page):
3021
url = f"http://legacy.python.org{path}"
3122
# Retrieve the image
@@ -34,27 +25,18 @@ def fix_image(self, path, page):
3425
if r.status_code != HTTP_OK:
3526
return None
3627

37-
# Create new associated image and generate ultimate path
28+
# Extract and validate filename (alphanumeric, hyphens, dots only)
29+
raw_name = PurePosixPath(urlparse(url).path).name
30+
filename = re.sub(r"[^\w.\-]", "_", raw_name)
31+
if not filename or filename.startswith("."):
32+
return None
33+
34+
# Use Django's storage API to safely write the file
3835
img = Image()
3936
img.page = page
37+
img.image.save(filename, ContentFile(r.content), save=True)
4038

41-
filename = Path(urlparse(url).path).name
42-
output_path = page_image_path(img, filename)
43-
44-
# Make sure our directories exist
45-
directory = Path(output_path).parent
46-
directory.mkdir(parents=True, exist_ok=True)
47-
48-
# Write image data to our location
49-
with Path(output_path).open("wb") as f:
50-
f.write(r.content)
51-
52-
# Re-open the image as a Django File object
53-
with Path(output_path).open("rb") as reopen:
54-
new_file = File(reopen)
55-
img.image.save(filename, new_file, save=True)
56-
57-
return self.image_url(output_path)
39+
return img.image.url
5840

5941
def find_image_paths(self, page):
6042
content = page.content.raw

apps/sponsors/management/commands/create_pycon_vouchers_for_sponsors.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def api_call(uri, query):
5151

5252
headers = {
5353
"X-API-Key": str(settings.PYCON_API_KEY),
54-
"X-API-Signature": str(sha1(base_string.encode("utf-8")).hexdigest()), # noqa: S324 - API signature, not for security storage
54+
"X-API-Signature": str(sha1(base_string.encode("utf-8")).hexdigest()), # noqa: S324 - required by PyCon API
5555
"X-API-Timestamp": str(timestamp),
5656
}
5757
scheme = "http" if settings.DEBUG else "https"

pydotorg/views.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import json
55
import re
66
from collections import defaultdict
7-
from pathlib import Path
7+
from pathlib import Path, PurePosixPath
88

99
from django.conf import settings
1010
from django.http import HttpResponse, JsonResponse
@@ -84,7 +84,11 @@ class MediaMigrationView(RedirectView):
8484

8585
def get_redirect_url(self, *args, **kwargs):
8686
"""Build the S3 redirect URL from the media path."""
87-
image_path = kwargs["url"]
87+
# Normalize path to prevent traversal (../, ....//, %2e%2e/, etc.)
88+
image_path = PurePosixPath(kwargs["url"]).as_posix().lstrip("/")
89+
# Collapse any remaining parent references after normalization
90+
parts = [p for p in image_path.split("/") if p not in (".", "..")]
91+
image_path = "/".join(parts)
8892
if self.prefix:
8993
image_path = f"{self.prefix}/{image_path}"
9094
return f"{settings.AWS_S3_ENDPOINT_URL}/{settings.AWS_STORAGE_BUCKET_NAME}/{image_path}"

static/fonts/demo/demo.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ document.body.addEventListener("click", function(e) {
1515
testDrive = document.getElementById('testDrive'),
1616
testText = document.getElementById('testText');
1717
function updateTest() {
18-
testDrive.innerHTML = testText.value || String.fromCharCode(160);
18+
testDrive.textContent = testText.value || String.fromCharCode(160);
1919
if (window.icomoonLiga) {
2020
window.icomoonLiga(testDrive);
2121
}

static/js/sponsors/applicationForm.js

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
const DESKTOP_WIDTH_LIMIT = 1200;
22

3+
function isSafeImageSrc(url) {
4+
if (!url) return false;
5+
// Allow relative URLs starting with / (but not protocol-relative //)
6+
if (url.charAt(0) === '/' && url.charAt(1) !== '/') return true;
7+
try {
8+
let parsed = new URL(url, window.location.origin);
9+
return parsed.origin === window.location.origin;
10+
} catch (e) {
11+
return false;
12+
}
13+
}
14+
315
$(document).ready(function(){
416
const SELECTORS = {
517
packageInput: function() { return $("input[name=package]"); },
@@ -66,7 +78,9 @@ $(document).ready(function(){
6678
img.setAttribute('data-next-state', src);
6779
}
6880

69-
img.setAttribute('src', initImg);
81+
if (isSafeImageSrc(initImg)) {
82+
img.setAttribute('src', initImg);
83+
}
7084
});
7185
$(".selected").removeClass("selected");
7286
$('.custom-fee').hide();
@@ -89,7 +103,9 @@ $(document).ready(function(){
89103
img.setAttribute('data-next-state', src);
90104
}
91105

92-
img.setAttribute('src', initImg);
106+
if (isSafeImageSrc(initImg)) {
107+
img.setAttribute('src', initImg);
108+
}
93109
});
94110
$(".selected").removeClass("selected");
95111

@@ -166,7 +182,9 @@ function benefitUpdate(benefitId, packageId) {
166182
const benefitsInputs = Array(...document.querySelectorAll('[data-benefit-id]'));
167183
const hiddenInput = benefitsInputs.filter((b) => b.getAttribute('data-benefit-id') == benefitId)[0];
168184
hiddenInput.checked = !hiddenInput.checked;
169-
clickedImg.src = newSrc;
185+
if (isSafeImageSrc(newSrc)) {
186+
clickedImg.src = newSrc;
187+
}
170188

171189
// Check if there are any type of customization. If so, display custom-fee label.
172190
let pkgBenefits = Array(...document.getElementById(`package_benefits_${packageId}`).children).map(div => div.textContent).sort();

0 commit comments

Comments
 (0)