diff --git a/common/static/js/vendor/pdfjs/viewer.js b/common/static/js/vendor/pdfjs/viewer.js index bfa90d05a782..b82b9d28084c 100644 --- a/common/static/js/vendor/pdfjs/viewer.js +++ b/common/static/js/vendor/pdfjs/viewer.js @@ -27,7 +27,7 @@ 'use strict'; -var DEFAULT_URL = 'compressed.tracemonkey-pldi-09.pdf'; +var DEFAULT_URL = ''; var DEFAULT_SCALE_DELTA = 1.1; var MIN_SCALE = 0.25; var MAX_SCALE = 10.0; @@ -1291,7 +1291,7 @@ var PDFFindController = (function PDFFindControllerClosure() { if (this.pagesToSearch < 0) { // No point in wrapping again, there were no matches. this.updateMatch(false); - // while matches were not found, searching for a page + // while matches were not found, searching for a page // with matches should nevertheless halt. return true; } @@ -1324,7 +1324,7 @@ var PDFFindController = (function PDFFindControllerClosure() { offset.matchIdx = null; this.pagesToSearch--; - + if (offset.pageIdx >= numPages || offset.pageIdx < 0) { offset.pageIdx = (previous ? numPages - 1 : 0); offset.wrapped = true; @@ -1335,7 +1335,7 @@ var PDFFindController = (function PDFFindControllerClosure() { var state = FindStates.FIND_NOTFOUND; var wrapped = this.offset.wrapped; this.offset.wrapped = false; - + if (found) { var previousPage = this.selected.pageIdx; this.selected.pageIdx = this.offset.pageIdx; @@ -1346,7 +1346,7 @@ var PDFFindController = (function PDFFindControllerClosure() { this.updatePage(previousPage); } } - + this.updateUIState(state, this.state.findPrevious); if (this.selected.pageIdx !== -1) { this.updatePage(this.selected.pageIdx); @@ -2708,7 +2708,7 @@ var DocumentProperties = { parseDate: function documentPropertiesParseDate(inputDate) { // This is implemented according to the PDF specification (see - // http://www.gnupdf.org/Date for an overview), but note that + // http://www.gnupdf.org/Date for an overview), but note that // Adobe Reader doesn't handle changing the date to universal time // and doesn't use the user's time zone (they're effectively ignoring // the HH' and mm' parts of the date string). diff --git a/lms/djangoapps/staticbook/tests.py b/lms/djangoapps/staticbook/tests.py index 919ee16c4fef..1d42ac4b5bcc 100644 --- a/lms/djangoapps/staticbook/tests.py +++ b/lms/djangoapps/staticbook/tests.py @@ -129,8 +129,11 @@ def test_book(self): url = self.make_url('pdf_book', book_index=0) response = self.client.get(url) self.assertContains(response, "Chapter 1 for PDF") - self.assertNotContains(response, "options.chapterNum =") - self.assertNotContains(response, "page=") + # Verify file parameter is not present (security fix) + self.assertNotContains(response, "file=") + # Verify postMessage infrastructure is in place + self.assertContains(response, "request_pdf_url") + self.assertContains(response, "pdf_url_response") def test_book_chapter(self): # We can access a book at a particular chapter. @@ -138,8 +141,10 @@ def test_book_chapter(self): url = self.make_url('pdf_book', book_index=0, chapter=2) response = self.client.get(url) self.assertContains(response, "Chapter 2 for PDF") - self.assertContains(response, "file={}".format(PDF_BOOK['chapters'][1]['url'])) - self.assertNotContains(response, "page=") + # Verify file parameter is not present anywhere (security fix) + self.assertNotContains(response, "file=") + # Verify postMessage infrastructure is in place + self.assertContains(response, "request_pdf_url") def test_book_page(self): # We can access a book at a particular page. @@ -147,7 +152,9 @@ def test_book_page(self): url = self.make_url('pdf_book', book_index=0, page=17) response = self.client.get(url) self.assertContains(response, "Chapter 1 for PDF") - self.assertNotContains(response, "options.chapterNum =") + # Verify file parameter is not present (security fix) + self.assertNotContains(response, "file=") + # Page parameter is still used in viewer_params self.assertContains(response, "page=17") def test_book_chapter_page(self): @@ -156,7 +163,9 @@ def test_book_chapter_page(self): url = self.make_url('pdf_book', book_index=0, chapter=2, page=17) response = self.client.get(url) self.assertContains(response, "Chapter 2 for PDF") - self.assertContains(response, "file={}".format(PDF_BOOK['chapters'][1]['url'])) + # Verify file parameter is not present (security fix) + self.assertNotContains(response, "file=") + # Page parameter is still used in viewer_params self.assertContains(response, "page=17") def test_bad_book_id(self): @@ -202,29 +211,32 @@ def test_chapter_page_xss(self): def test_static_url_map_contentstore(self): """ - This ensure static URL mapping is happening properly for - a course that uses the contentstore + This ensure static URL mapping is happening properly for + a course that uses the contentstore. + URLs are remapped in backend but not exposed via file parameter (security fix). """ self.make_course(pdf_textbooks=[PORTABLE_PDF_BOOK]) url = self.make_url('pdf_book', book_index=0, chapter=1) response = self.client.get(url) - self.assertNotContains(response, 'file={}'.format(PORTABLE_PDF_BOOK['chapters'][0]['url'])) - self.assertContains(response, 'file=/asset-v1:{0.org}+{0.course}+{0.run}+type@asset+block/{1}'.format( + # Verify file parameter is not present in response (security fix) + self.assertNotContains(response, 'file=') + # Verify the chapter URL is in the sidebar for postMessage communication + self.assertContains(response, '/asset-v1:{0.org}+{0.course}+{0.run}+type@asset+block/{1}'.format( self.course.location, PORTABLE_PDF_BOOK['chapters'][0]['url'].replace('/static/', ''))) def test_static_url_map_static_asset_path(self): """ - Like above, but used when the course has set a static_asset_path + Like above, but used when the course has set a static_asset_path. + URLs are remapped in backend but not exposed via file parameter (security fix). """ self.make_course(pdf_textbooks=[PORTABLE_PDF_BOOK], static_asset_path='awesomesauce') url = self.make_url('pdf_book', book_index=0, chapter=1) response = self.client.get(url) - self.assertNotContains(response, 'file={}'.format(PORTABLE_PDF_BOOK['chapters'][0]['url'])) - self.assertNotContains(response, 'file=/c4x/{0.org}/{0.course}/asset/{1}'.format( - self.course.location, - PORTABLE_PDF_BOOK['chapters'][0]['url'].replace('/static/', ''))) - self.assertContains(response, 'file=/static/awesomesauce/{}'.format( + # Verify file parameter is not present anywhere (security fix) + self.assertNotContains(response, 'file=') + # Verify the remapped URL is in the sidebar for postMessage communication + self.assertContains(response, '/static/awesomesauce/{}'.format( PORTABLE_PDF_BOOK['chapters'][0]['url'].replace('/static/', ''))) def test_invalid_chapter_id(self): diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 2e6d1c9a8ed5..c33da4225de4 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -86,7 +86,7 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): raise Http404(f"Invalid book index value: {book_index}") textbook = course.pdf_textbooks[book_index] - viewer_params = '&file=' + viewer_params = '' current_url = '' if 'url' in textbook: @@ -99,6 +99,9 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): if 'chapters' in textbook: for entry in textbook['chapters']: entry['url'] = remap_static_url(entry['url'], course) + # Security: Validate chapter URL doesn't contain dangerous schemes + if entry['url'].lower().startswith(('javascript:', 'data:', 'vbscript:', 'file:')): + entry['url'] = '' # Sanitize dangerous URLs if chapter is not None and int(chapter) <= (len(textbook['chapters'])): current_chapter = textbook['chapters'][int(chapter) - 1] else: diff --git a/lms/templates/pdf_viewer.html b/lms/templates/pdf_viewer.html index a3314d54b5ac..1f4b8cd54abf 100644 --- a/lms/templates/pdf_viewer.html +++ b/lms/templates/pdf_viewer.html @@ -1,5 +1,5 @@ <%page expression_filter="h"/> - + <%namespace name='static' file='static_content.html'/> <%! from openedx.core.djangolib.js_utils import ( @@ -46,10 +46,44 @@ PDFJS.workerSrc = "${static.url('js/vendor/pdfjs/pdf.worker.js') | n, js_escaped_string}"; PDFJS.disableWorker = true; PDFJS.cMapUrl = "${static.url('css/vendor/pdfjs/cmaps/') | n, js_escaped_string}"; - PDF_URL = '${current_url | n, js_escaped_string}'; + + var PDF_URL = '${current_url | n, js_escaped_string}'; + + if (window.parent !== window) { + window.parent.postMessage({type: 'request_pdf_url'}, '*'); + + function handlePdfUrlResponse(event) { + if (event.data && event.data.type === 'pdf_url_response') { + PDF_URL = event.data.url; + + if (PDFViewerApplication.open) { + PDFViewerApplication.open(PDF_URL); + PDFViewerApplication.mouseScroll(0); + + setTimeout(function() { + if (PDFViewerApplication.pdfDocument) { + if (event.data.title) document.getElementById('titleField').textContent = event.data.title; + if (event.data.author) document.getElementById('authorField').textContent = event.data.author; + if (event.data.subject) document.getElementById('subjectField').textContent = event.data.subject; + if (event.data.keywords) document.getElementById('keywordsField').textContent = event.data.keywords; + document.getElementById('creatorField').textContent = 'edX Platform'; + } + }, 500); + } + } else if (event.data && event.data.type === 'chapter_change') { + window.parent.postMessage({type: 'request_pdf_url'}, '*'); + } + } + + window.addEventListener('message', handlePdfUrlResponse); + } + + document.addEventListener('DOMContentLoaded', function () { + PDFViewerApplication && PDFViewerApplication.open(PDF_URL); + }); - + <%static:js group='main_vendor'/> <%static:js group='application'/> @@ -347,77 +381,77 @@
-