Skip to content

Commit 7e3e394

Browse files
papphelixpapphelix
committed
fix: enhance PDF viewer security by removing file param and communicating via postMessage
1 parent ea35076 commit 7e3e394

3 files changed

Lines changed: 149 additions & 150 deletions

File tree

‎lms/djangoapps/staticbook/views.py‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,30 +81,17 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
8181
course = get_course_with_access(request.user, 'load', course_key)
8282
staff_access = bool(has_access(request.user, 'staff', course))
8383

84-
# Security: Check if file parameter contains external URL or dangerous schemes and reject it
85-
file_param = request.GET.get('file', '')
86-
if file_param:
87-
# Block external URLs
88-
if file_param.startswith(('http://', 'https://')):
89-
raise Http404("External URLs are not allowed in file parameter")
90-
# Block dangerous URL schemes (XSS vectors)
91-
if file_param.lower().startswith(('javascript:', 'data:', 'vbscript:', 'file:')):
92-
raise Http404("Dangerous URL scheme detected in file parameter")
93-
9484
book_index = int(book_index)
9585
if book_index < 0 or book_index >= len(course.pdf_textbooks):
9686
raise Http404(f"Invalid book index value: {book_index}")
9787
textbook = course.pdf_textbooks[book_index]
9888

99-
viewer_params = ''
89+
viewer_params = '#zoom=page-fit&disableRange=true'
10090
current_url = ''
10191

10292
if 'url' in textbook:
10393
textbook['url'] = remap_static_url(textbook['url'], course)
10494
current_url = textbook['url']
105-
if not current_url.startswith(('http://', 'https://')):
106-
viewer_params = '&file='
107-
viewer_params += current_url
10895

10996
# then remap all the chapter URLs as well, if they are provided.
11097
current_chapter = None
@@ -120,9 +107,6 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
120107
current_chapter = textbook['chapters'][0]
121108

122109
current_url = current_chapter['url']
123-
if not current_url.startswith(('http://', 'https://')):
124-
viewer_params = '&file='
125-
viewer_params += current_url
126110

127111
viewer_params += '#zoom=page-fit&disableRange=true'
128112
if page is not None:

‎lms/templates/pdf_viewer.html‎

Lines changed: 104 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<%page expression_filter="h"/>
2-
<!DOCTYPE html>
2+
<!DOCTYPE html>
33
<%namespace name='static' file='static_content.html'/>
44
<%!
55
from openedx.core.djangolib.js_utils import (
@@ -46,10 +46,41 @@
4646
PDFJS.workerSrc = "${static.url('js/vendor/pdfjs/pdf.worker.js') | n, js_escaped_string}";
4747
PDFJS.disableWorker = true;
4848
PDFJS.cMapUrl = "${static.url('css/vendor/pdfjs/cmaps/') | n, js_escaped_string}";
49-
PDF_URL = '${current_url | n, js_escaped_string}';
49+
50+
var PDF_URL = '${current_url | n, js_escaped_string}';
51+
52+
if (window.parent !== window) {
53+
window.parent.postMessage({type: 'request_pdf_url'}, '*');
54+
55+
function handlePdfUrlResponse(event) {
56+
if (event.data && event.data.type === 'pdf_url_response') {
57+
PDF_URL = event.data.url;
58+
59+
if (PDFViewerApplication.open) {
60+
PDFViewerApplication.open(PDF_URL);
61+
PDFViewerApplication.mouseScroll(0);
62+
63+
setTimeout(function() {
64+
if (PDFViewerApplication.pdfDocument && PDFViewerApplication.pdfDocument.documentInfo) {
65+
var info = PDFViewerApplication.pdfDocument.documentInfo;
66+
if (event.data.title) info.Title = event.data.title;
67+
if (event.data.author) info.Author = event.data.author;
68+
if (event.data.subject) info.Subject = event.data.subject;
69+
if (event.data.keywords) info.Keywords = event.data.keywords;
70+
info.Creator = 'edX Platform';
71+
}
72+
}, 500);
73+
}
74+
} else if (event.data && event.data.type === 'chapter_change') {
75+
window.parent.postMessage({type: 'request_pdf_url'}, '*');
76+
}
77+
}
78+
79+
window.addEventListener('message', handlePdfUrlResponse);
80+
}
5081
</script>
5182

52-
<script ${static.url('js/vendor/pdfjs/debugger.js')}></script>
83+
<script type="text/javascript" src="${static.url('js/vendor/pdfjs/debugger.js')}"></script>
5384

5485
<%static:js group='main_vendor'/>
5586
<%static:js group='application'/>
@@ -347,77 +378,77 @@
347378

348379
</div> <!-- outerContainer -->
349380
<div id="printContainer"></div>
350-
<div id="mozPrintCallback-shim" hidden>
351-
<style scoped>
352-
#mozPrintCallback-shim {
353-
position: fixed;
354-
top: 0;
355-
left: 0;
356-
height: 100%;
357-
width: 100%;
358-
z-index: 9999999;
359-
360-
display: block;
361-
text-align: center;
362-
background-color: rgba(0, 0, 0, 0.5);
363-
}
364-
#mozPrintCallback-shim[hidden] {
365-
display: none;
366-
}
367-
@media print {
368-
#mozPrintCallback-shim {
369-
display: none;
370-
}
371-
}
372-
373-
#mozPrintCallback-shim .mozPrintCallback-dialog-box {
374-
display: inline-block;
375-
margin: -50px auto 0;
376-
position: relative;
377-
top: 45%;
378-
left: 0;
379-
min-width: 220px;
380-
max-width: 400px;
381-
382-
padding: 9px;
383-
384-
border: 1px solid hsla(0, 0%, 0%, .5);
385-
border-radius: 2px;
386-
box-shadow: 0 1px 4px rgba(0, 0, 0, 0.3);
387-
388-
background-color: #474747;
389-
390-
color: hsl(0, 0%, 85%);
391-
font-size: 16px;
392-
line-height: 20px;
393-
}
394-
#mozPrintCallback-shim .progress-row {
395-
clear: both;
396-
padding: 1em 0;
397-
}
398-
#mozPrintCallback-shim progress {
399-
width: 100%;
400-
}
401-
#mozPrintCallback-shim .relative-progress {
402-
clear: both;
403-
float: right;
404-
}
405-
#mozPrintCallback-shim .progress-actions {
406-
clear: both;
407-
}
408-
</style>
409-
<div class="mozPrintCallback-dialog-box">
410-
<!-- TODO: Localise the following strings -->
411-
Preparing document for printing...
412-
<div class="progress-row">
413-
<progress value="0" max="100"></progress>
414-
<span class="relative-progress">0%</span>
415-
</div>
416-
<div class="progress-actions">
417-
<input type="button" value="Cancel" class="mozPrintCallback-cancel">
381+
<div id="mozPrintCallback-shim" hidden>
382+
<style scoped>
383+
#mozPrintCallback-shim {
384+
position: fixed;
385+
top: 0;
386+
left: 0;
387+
height: 100%;
388+
width: 100%;
389+
z-index: 9999999;
390+
391+
display: block;
392+
text-align: center;
393+
background-color: rgba(0, 0, 0, 0.5);
394+
}
395+
#mozPrintCallback-shim[hidden] {
396+
display: none;
397+
}
398+
@media print {
399+
#mozPrintCallback-shim {
400+
display: none;
401+
}
402+
}
403+
404+
#mozPrintCallback-shim .mozPrintCallback-dialog-box {
405+
display: inline-block;
406+
margin: -50px auto 0;
407+
position: relative;
408+
top: 45%;
409+
left: 0;
410+
min-width: 220px;
411+
max-width: 400px;
412+
413+
padding: 9px;
414+
415+
border: 1px solid hsla(0, 0%, 0%, .5);
416+
border-radius: 2px;
417+
box-shadow: 0 1px 4px rgba(0, 0, 0, 0.3);
418+
419+
background-color: #474747;
420+
421+
color: hsl(0, 0%, 85%);
422+
font-size: 16px;
423+
line-height: 20px;
424+
}
425+
#mozPrintCallback-shim .progress-row {
426+
clear: both;
427+
padding: 1em 0;
428+
}
429+
#mozPrintCallback-shim progress {
430+
width: 100%;
431+
}
432+
#mozPrintCallback-shim .relative-progress {
433+
clear: both;
434+
float: right;
435+
}
436+
#mozPrintCallback-shim .progress-actions {
437+
clear: both;
438+
}
439+
</style>
440+
<div class="mozPrintCallback-dialog-box">
441+
<!-- TODO: Localise the following strings -->
442+
Preparing document for printing...
443+
<div class="progress-row">
444+
<progress value="0" max="100"></progress>
445+
<span class="relative-progress">0%</span>
446+
</div>
447+
<div class="progress-actions">
448+
<input type="button" value="Cancel" class="mozPrintCallback-cancel">
449+
</div>
450+
</div>
418451
</div>
419-
</div>
420-
</div>
421452
<script type="text/javascript" src="${static.url('js/vendor/pdfjs/viewer.js')}"></script>
422453
<script type="text/javascript" src="${static.url('js/pdf-analytics.js')}"></script>
423454
</body>

‎lms/templates/static_pdfbook.html‎

Lines changed: 44 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -19,79 +19,63 @@
1919
<%include file="/courseware/course_navigation.html" args="active_page='pdftextbook/{0}'.format(book_index)" />
2020
<script>
2121
$(function(){
22-
function sanitizeUrl(url) {
23-
var temp = $('<div>').text(url).html();
24-
if (/^(javascript|data|vbscript|file):/i.test(temp)) {
25-
return '';
26-
}
27-
// Check for external URLs
28-
if (temp.startsWith('http://') || temp.startsWith('https://')) {
29-
return '';
30-
}
31-
return temp;
32-
}
22+
var currentChapterUrl = null;
23+
var currentChapterTitle = null;
3324

34-
function isDangerousSrc(src) {
35-
if (!src) return false;
36-
// Check for dangerous protocols
37-
if (/^(javascript|data|vbscript|file):/i.test(src)) {
38-
return true;
39-
}
40-
// Check if file parameter contains dangerous content
41-
try {
42-
var url = new URL(src, window.location.origin);
43-
var fileParam = url.searchParams.get('file');
44-
if (fileParam) {
45-
if (/^(javascript|data|vbscript|file|http:\/\/|https:\/\/)/i.test(fileParam)) {
46-
return true;
47-
}
25+
// Handle PDF URL requests from iframe (PostMessage API requires 'message' event type)
26+
function handlePdfUrlRequest(event) {
27+
// Verify this is a PDF URL request from our iframe
28+
if (event.data && event.data.type === 'request_pdf_url') {
29+
if (currentChapterUrl) {
30+
// Send the selected chapter URL back to the PDF viewer iframe
31+
event.source.postMessage({
32+
type: 'pdf_url_response',
33+
url: currentChapterUrl,
34+
title: currentChapterTitle || '',
35+
author: 'edX Course',
36+
subject: 'Course Material',
37+
keywords: 'education,textbook,course'
38+
}, '*');
4839
}
49-
} catch (e) {
50-
return true;
5140
}
52-
return false;
5341
}
5442

55-
// Security: Monitor iframe src changes to prevent XSS via inspect element
56-
var iframe = $('#viewer-frame')[0];
57-
if (iframe) {
58-
var observer = new MutationObserver(function(mutations) {
59-
mutations.forEach(function(mutation) {
60-
if (mutation.type === 'attributes' && mutation.attributeName === 'src') {
61-
var currentSrc = iframe.getAttribute('src');
62-
if (isDangerousSrc(currentSrc)) {
63-
iframe.setAttribute('src', '');
64-
}
65-
}
66-
});
67-
});
68-
69-
observer.observe(iframe, {
70-
attributes: true,
71-
attributeFilter: ['src']
72-
});
73-
74-
// Also validate initial src
75-
var initialSrc = iframe.getAttribute('src');
76-
if (isDangerousSrc(initialSrc)) {
77-
iframe.setAttribute('src', '');
78-
}
79-
}
43+
// Listen for PostMessage communication from PDF viewer iframe
44+
window.addEventListener('message', handlePdfUrlRequest);
8045

8146
$('.chapter').click(function(e){
8247
e.preventDefault();
83-
var url = sanitizeUrl($(this).attr('rel'));
84-
if (!url) {
48+
var $this = $(this);
49+
var url = $this.attr('rel');
50+
var chapterTitle = $this.text();
51+
52+
// Store the current chapter URL for postMessage
53+
currentChapterUrl = url;
54+
currentChapterTitle = chapterTitle;
55+
56+
if ($('#viewer-frame').attr('src').length !== 0) {
57+
$('#viewer-frame')[0].contentWindow.postMessage({type: 'chapter_change'}, '*');
58+
Logger.log("textbook.pdf.chapter.navigated", {
59+
"name": "textbook.pdf.chapter.navigated",
60+
"chapter": url,
61+
"chapter_title": chapterTitle
62+
});
8563
return;
8664
}
87-
var title = $('<div>').text($(this).text()).html();
65+
66+
// Load iframe without file parameter - secure approach (first time only)
8867
$('#viewer-frame').attr({
89-
'src': '${request.path | n, js_escaped_string}?viewer=true&file=' + encodeURIComponent(url) + '#zoom=page-fit&disableRange=true',
90-
'title': title
91-
});
68+
'src': '${request.path | n, js_escaped_string}?viewer=true#zoom=page-fit&disableRange=true',
69+
'title': chapterTitle
70+
});
9271
$('#viewer-frame').focus();
93-
Logger.log("textbook.pdf.chapter.navigated", {"name": "textbook.pdf.chapter.navigated", "chapter": url, "chapter_title": title});
72+
73+
Logger.log("textbook.pdf.chapter.navigated", {
74+
"name": "textbook.pdf.chapter.navigated",
75+
"chapter": url,
76+
"chapter_title": chapterTitle
9477
});
78+
});
9579
});
9680
</script>
9781

0 commit comments

Comments
 (0)