fix: restrict PDF rendering to relative paths

lms/djangoapps/staticbook/tests.py

@@ -129,35 +129,44 @@ 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.
         self.make_course(pdf_textbooks=[PDF_BOOK])
         url = self.make_url('pdf_book', book_index=0, chapter=2)
         response = self.client.get(url)
         self.assertContains(response, "Chapter 2 for PDF")
-        self.assertNotContains(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.
         self.make_course(pdf_textbooks=[PDF_BOOK])
         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 =")
-        self.assertNotContains(response, "page=17")
+        # 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):
         # We can access a book at a particular chapter and page.
         self.make_course(pdf_textbooks=[PDF_BOOK])
         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.assertNotContains(response, "file={}".format(PDF_BOOK['chapters'][1]['url']))
-        self.assertNotContains(response, "page=17")
+        # 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):
         # If the book id isn't an int, we'll get a 404.
@@ -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):

lms/djangoapps/staticbook/views.py

@@ -92,24 +92,21 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
     if 'url' in textbook:
         textbook['url'] = remap_static_url(textbook['url'], course)
         current_url = textbook['url']
-        if not current_url.startswith(('http://', 'https://')):
-            viewer_params = '&file='
-            viewer_params += current_url
 
     # then remap all the chapter URLs as well, if they are provided.
     current_chapter = 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:
             current_chapter = textbook['chapters'][0]
 
         current_url = current_chapter['url']
-        if not current_url.startswith(('http://', 'https://')):
-            viewer_params = '&file='
-            viewer_params += current_url
 
     viewer_params += '#zoom=page-fit&disableRange=true'
     if page is not None:

lms/templates/pdf_viewer.html

@@ -1,5 +1,5 @@
 <%page expression_filter="h"/>
-<!DOCTYPE html>
+<!DOCTYPE html>
 <%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);
+        });
     </script>
 
-    <script ${static.url('js/vendor/pdfjs/debugger.js')}></script>
+    <script type="text/javascript" src="${static.url('js/vendor/pdfjs/debugger.js')}"></script>
 
     <%static:js group='main_vendor'/>
     <%static:js group='application'/>
@@ -347,77 +381,77 @@
 
     </div> <!-- outerContainer -->
     <div id="printContainer"></div>
-<div id="mozPrintCallback-shim" hidden>
-  <style scoped>
-#mozPrintCallback-shim {
-  position: fixed;
-  top: 0;
-  left: 0;
-  height: 100%;
-  width: 100%;
-  z-index: 9999999;
-
-  display: block;
-  text-align: center;
-  background-color: rgba(0, 0, 0, 0.5);
-}
-#mozPrintCallback-shim[hidden] {
-  display: none;
-}
-@media print {
-  #mozPrintCallback-shim {
-    display: none;
-  }
-}
-
-#mozPrintCallback-shim .mozPrintCallback-dialog-box {
-  display: inline-block;
-  margin: -50px auto 0;
-  position: relative;
-  top: 45%;
-  left: 0;
-  min-width: 220px;
-  max-width: 400px;
-
-  padding: 9px;
-
-  border: 1px solid hsla(0, 0%, 0%, .5);
-  border-radius: 2px;
-  box-shadow: 0 1px 4px rgba(0, 0, 0, 0.3);
-
-  background-color: #474747;
-
-  color: hsl(0, 0%, 85%);
-  font-size: 16px;
-  line-height: 20px;
-}
-#mozPrintCallback-shim .progress-row {
-  clear: both;
-  padding: 1em 0;
-}
-#mozPrintCallback-shim progress {
-  width: 100%;
-}
-#mozPrintCallback-shim .relative-progress {
-  clear: both;
-  float: right;
-}
-#mozPrintCallback-shim .progress-actions {
-  clear: both;
-}
-  </style>
-  <div class="mozPrintCallback-dialog-box">
-    <!-- TODO: Localise the following strings -->
-    Preparing document for printing...
-    <div class="progress-row">
-      <progress value="0" max="100"></progress>
-      <span class="relative-progress">0%</span>
-    </div>
-    <div class="progress-actions">
-      <input type="button" value="Cancel" class="mozPrintCallback-cancel">
+    <div id="mozPrintCallback-shim" hidden>
+      <style scoped>
+        #mozPrintCallback-shim {
+          position: fixed;
+          top: 0;
+          left: 0;
+          height: 100%;
+          width: 100%;
+          z-index: 9999999;
+
+          display: block;
+          text-align: center;
+          background-color: rgba(0, 0, 0, 0.5);
+        }
+        #mozPrintCallback-shim[hidden] {
+          display: none;
+        }
+        @media print {
+          #mozPrintCallback-shim {
+            display: none;
+          }
+        }
+
+        #mozPrintCallback-shim .mozPrintCallback-dialog-box {
+          display: inline-block;
+          margin: -50px auto 0;
+          position: relative;
+          top: 45%;
+          left: 0;
+          min-width: 220px;
+          max-width: 400px;
+
+          padding: 9px;
+
+          border: 1px solid hsla(0, 0%, 0%, .5);
+          border-radius: 2px;
+          box-shadow: 0 1px 4px rgba(0, 0, 0, 0.3);
+
+          background-color: #474747;
+
+          color: hsl(0, 0%, 85%);
+          font-size: 16px;
+          line-height: 20px;
+        }
+        #mozPrintCallback-shim .progress-row {
+          clear: both;
+          padding: 1em 0;
+        }
+        #mozPrintCallback-shim progress {
+          width: 100%;
+        }
+        #mozPrintCallback-shim .relative-progress {
+          clear: both;
+          float: right;
+        }
+        #mozPrintCallback-shim .progress-actions {
+          clear: both;
+        }
+      </style>
+      <div class="mozPrintCallback-dialog-box">
+        <!-- TODO: Localise the following strings -->
+        Preparing document for printing...
+        <div class="progress-row">
+          <progress value="0" max="100"></progress>
+          <span class="relative-progress">0%</span>
+        </div>
+        <div class="progress-actions">
+          <input type="button" value="Cancel" class="mozPrintCallback-cancel">
+        </div>
+      </div>
     </div>
-  </div>
-</div>
     <script type="text/javascript" src="${static.url('js/vendor/pdfjs/viewer.js')}"></script>
     <script type="text/javascript" src="${static.url('js/pdf-analytics.js')}"></script>
   </body>