Skip to content

Commit 241b914

Browse files
feanilclaude
andcommitted
fix: prevent SSRF in the Studio video download endpoint
The PUT /api/contentstore/v1/videos/{course_id}/download endpoint fetched every client-supplied files[].url server-side with requests.get(url, allow_redirects=True) and returned the bytes inside the ZIP response. Because the URLs were never validated, an authenticated user with studio read access could point them at internal services or cloud metadata endpoints and exfiltrate the responses (GHSA-fpf9-9rpr-jvrx). By design these URLs are always a subset of the course's own VAL encoded_videos[].url values (the same data the video listing hands the frontend). Restrict fetches to that allowlist: build the set of legitimate URLs for the course and reject any request containing a URL outside it before any HTTP request is made. This eliminates the SSRF rather than merely narrowing it. Adds VideoDownloadViewTest (the endpoint previously had no test coverage) covering the allowed-URL success path, rejection of disallowed URLs without any outbound request, mixed allowed/disallowed batches, and the non-staff permission gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 723be60 commit 241b914

2 files changed

Lines changed: 141 additions & 1 deletion

File tree

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_videos.py

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,25 @@
11
"""
22
Unit tests for course settings views.
33
"""
4-
from unittest.mock import patch
4+
from datetime import datetime
5+
from unittest.mock import MagicMock, patch
56

67
import ddt
8+
import pytz
79
from django.conf import settings
810
from django.contrib.staticfiles.storage import staticfiles_storage
911
from django.urls import reverse
1012
from edx_toggles.toggles import WaffleSwitch
1113
from edx_toggles.toggles.testutils import override_waffle_switch
1214
from edxval.api import (
15+
create_profile,
16+
create_video,
1317
get_3rd_party_transcription_plans,
1418
get_transcript_credentials_state_for_org,
1519
get_transcript_preferences,
1620
)
1721
from rest_framework import status
22+
from rest_framework.test import APIClient
1823

1924
from cms.djangoapps.contentstore.video_storage_handlers import get_all_transcript_languages
2025
from cms.djangoapps.contentstore.tests.utils import CourseTestCase
@@ -135,3 +140,107 @@ def test_VideoTranscriptEnabledFlag_enabled(self):
135140
response = self.client.get(self.url)
136141
self.assertIn("is_ai_translations_enabled", response.data)
137142
self.assertTrue(response.data["is_ai_translations_enabled"])
143+
144+
145+
146+
class VideoDownloadViewTest(CourseTestCase):
147+
"""
148+
Tests for VideoDownloadView.
149+
150+
The download endpoint fetches each requested ``files[].url`` server-side and
151+
returns the bytes inside a zip. Those URLs must therefore be restricted to
152+
the course's own video URLs, otherwise the endpoint is an SSRF primitive
153+
(see GHSA-fpf9-9rpr-jvrx).
154+
"""
155+
156+
ALLOWED_URL = "http://example.com/profile1/test.mp4"
157+
# An internal address an attacker might try to reach via SSRF.
158+
SSRF_URL = "http://169.254.169.254/latest/meta-data/"
159+
160+
def setUp(self):
161+
super().setUp()
162+
# reverse() with only course_id resolves to the download route (the
163+
# usage route with the same name additionally requires edx_video_id).
164+
self.url = reverse(
165+
"cms.djangoapps.contentstore:v1:video_usage",
166+
kwargs={"course_id": self.course.id},
167+
)
168+
self.api_client = APIClient()
169+
self.api_client.force_authenticate(user=self.user)
170+
create_profile("profile1")
171+
create_video({
172+
"edx_video_id": "test-video",
173+
"client_video_id": "test.mp4",
174+
"duration": 42.0,
175+
"status": "file_complete",
176+
"courses": [str(self.course.id)],
177+
"created": datetime.now(pytz.utc),
178+
"encoded_videos": [
179+
{
180+
"profile": "profile1",
181+
"url": self.ALLOWED_URL,
182+
"file_size": 1600,
183+
"bitrate": 100,
184+
},
185+
],
186+
})
187+
188+
@patch("cms.djangoapps.contentstore.video_storage_handlers.requests.get")
189+
def test_download_allowed_url(self, mock_get):
190+
"""A URL that belongs to the course's videos is fetched and zipped."""
191+
mock_get.return_value = MagicMock(
192+
content=b"video-bytes",
193+
headers={"Content-Type": "video/mp4"},
194+
)
195+
response = self.api_client.put(
196+
self.url,
197+
data={"files": [{"url": self.ALLOWED_URL, "name": "test.mp4"}]},
198+
format="json",
199+
)
200+
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
201+
mock_get.assert_called_once_with(self.ALLOWED_URL, allow_redirects=True)
202+
203+
@patch("cms.djangoapps.contentstore.video_storage_handlers.requests.get")
204+
def test_rejects_url_not_belonging_to_course(self, mock_get):
205+
"""
206+
A URL that is not one of the course's video URLs is rejected before any
207+
server-side request is made (SSRF protection).
208+
"""
209+
response = self.api_client.put(
210+
self.url,
211+
data={"files": [{"url": self.SSRF_URL, "name": "evil.txt"}]},
212+
format="json",
213+
)
214+
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) # noqa: PT009
215+
mock_get.assert_not_called()
216+
217+
@patch("cms.djangoapps.contentstore.video_storage_handlers.requests.get")
218+
def test_rejects_when_any_url_is_disallowed(self, mock_get):
219+
"""
220+
A request mixing an allowed URL with a disallowed one is rejected
221+
outright, without fetching the allowed URL either.
222+
"""
223+
response = self.api_client.put(
224+
self.url,
225+
data={"files": [
226+
{"url": self.ALLOWED_URL, "name": "test.mp4"},
227+
{"url": self.SSRF_URL, "name": "evil.txt"},
228+
]},
229+
format="json",
230+
)
231+
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) # noqa: PT009
232+
mock_get.assert_not_called()
233+
234+
@patch("cms.djangoapps.contentstore.video_storage_handlers.requests.get")
235+
def test_non_staff_user_denied(self, mock_get):
236+
"""A user without studio read access cannot reach the fetch path."""
237+
__, nonstaff_user = self.create_non_staff_authed_user_client()
238+
client = APIClient()
239+
client.force_authenticate(user=nonstaff_user)
240+
response = client.put(
241+
self.url,
242+
data={"files": [{"url": self.ALLOWED_URL, "name": "test.mp4"}]},
243+
format="json",
244+
)
245+
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) # noqa: PT009
246+
mock_get.assert_not_called()

cms/djangoapps/contentstore/video_storage_handlers.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
from path import Path as path
4848
from pytz import UTC
4949
from rest_framework import status as rest_status
50+
from rest_framework.exceptions import ValidationError
5051
from rest_framework.response import Response
5152
from tempfile import NamedTemporaryFile, mkdtemp
5253
from wsgiref.util import FileWrapper
@@ -242,6 +243,29 @@ def send_zip(zip_file, size=None):
242243
return response
243244

244245

246+
def get_course_video_download_urls(course_key_string):
247+
"""
248+
Return the set of encoded-video URLs that legitimately belong to the given
249+
course, as recorded in VAL.
250+
251+
The video download endpoint only ever needs to fetch URLs that were already
252+
surfaced to the client by the video listing. Restricting fetches to this set
253+
prevents server-side request forgery (SSRF) via attacker-supplied URLs.
254+
"""
255+
videos, __ = get_videos_for_course(
256+
course_key_string,
257+
VideoSortField.created,
258+
SortDirection.desc,
259+
None,
260+
)
261+
return {
262+
encoding['url']
263+
for video in videos
264+
for encoding in video['encoded_videos']
265+
if encoding.get('url')
266+
}
267+
268+
245269
def create_video_zip(course_key_string, files):
246270
"""
247271
Generates the video zip, or returns None if there was an error.
@@ -254,6 +278,13 @@ def create_video_zip(course_key_string, files):
254278
root_dir = path(mkdtemp())
255279
video_dir = root_dir + '/' + name
256280
zip_folder = None
281+
# Only allow fetching URLs that belong to this course's videos. Anything
282+
# else (internal services, cloud metadata endpoints, arbitrary hosts) is a
283+
# potential SSRF target and is rejected before any request is made.
284+
allowed_urls = get_course_video_download_urls(course_key_string)
285+
for file in files:
286+
if file['url'] not in allowed_urls:
287+
raise ValidationError(f"Invalid video download url: {file['url']}")
257288
try:
258289
for file in files:
259290
url = file['url']

0 commit comments

Comments
 (0)