Skip to content

Commit 00b7c3c

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 1b338e7 commit 00b7c3c

2 files changed

Lines changed: 140 additions & 1 deletion

File tree

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

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

5354
from common.djangoapps.util.json_request import JsonResponse
@@ -237,6 +238,29 @@ def send_zip(zip_file, size=None):
237238
return response
238239

239240

241+
def get_course_video_download_urls(course_key_string):
242+
"""
243+
Return the set of encoded-video URLs that legitimately belong to the given
244+
course, as recorded in VAL.
245+
246+
The video download endpoint only ever needs to fetch URLs that were already
247+
surfaced to the client by the video listing. Restricting fetches to this set
248+
prevents server-side request forgery (SSRF) via attacker-supplied URLs.
249+
"""
250+
videos, __ = get_videos_for_course(
251+
course_key_string,
252+
VideoSortField.created,
253+
SortDirection.desc,
254+
None,
255+
)
256+
return {
257+
encoding['url']
258+
for video in videos
259+
for encoding in video['encoded_videos']
260+
if encoding.get('url')
261+
}
262+
263+
240264
def create_video_zip(course_key_string, files):
241265
"""
242266
Generates the video zip, or returns None if there was an error.
@@ -249,6 +273,13 @@ def create_video_zip(course_key_string, files):
249273
root_dir = path(mkdtemp())
250274
video_dir = root_dir + '/' + name
251275
zip_folder = None
276+
# Only allow fetching URLs that belong to this course's videos. Anything
277+
# else (internal services, cloud metadata endpoints, arbitrary hosts) is a
278+
# potential SSRF target and is rejected before any request is made.
279+
allowed_urls = get_course_video_download_urls(course_key_string)
280+
for file in files:
281+
if file['url'] not in allowed_urls:
282+
raise ValidationError(f"Invalid video download url: {file['url']}")
252283
try:
253284
for file in files:
254285
url = file['url']

0 commit comments

Comments
 (0)