From ad03225cf6a679a4445e071c8c71fd8f182419bb Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Thu, 20 Feb 2014 15:46:44 -0800 Subject: [PATCH 1/5] Audit .html() calls take #2 --- IPython/html/static/notebook/js/codecell.js | 1 + IPython/html/static/notebook/js/outputarea.js | 17 ++++++----- IPython/html/static/notebook/js/pager.js | 2 ++ IPython/html/static/notebook/js/textcell.js | 28 ++++++++++--------- IPython/html/static/notebook/js/tooltip.js | 1 + 5 files changed, 29 insertions(+), 20 deletions(-) diff --git a/IPython/html/static/notebook/js/codecell.js b/IPython/html/static/notebook/js/codecell.js index 6a12f98a0..20b306a2d 100644 --- a/IPython/html/static/notebook/js/codecell.js +++ b/IPython/html/static/notebook/js/codecell.js @@ -473,6 +473,7 @@ var IPython = (function (IPython) { } this.input_prompt_number = number; var prompt_html = CodeCell.input_prompt_function(this.input_prompt_number, nline); + // This HTML call is okay because the user contents are escaped. this.element.find('div.input_prompt').html(prompt_html); }; diff --git a/IPython/html/static/notebook/js/outputarea.js b/IPython/html/static/notebook/js/outputarea.js index bc3bf0f32..17b41437e 100644 --- a/IPython/html/static/notebook/js/outputarea.js +++ b/IPython/html/static/notebook/js/outputarea.js @@ -343,7 +343,8 @@ var IPython = (function (IPython) { // Insert the subarea into the iframe // We must directly write the html. When using Jquery's append // method, javascript is evaluated in the parent document and - // not in the iframe document. + // not in the iframe document. At this point, subarea doesn't + // contain any user content. this.contentDocument.write(subarea.html()); this.contentDocument.close(); @@ -370,12 +371,10 @@ var IPython = (function (IPython) { // display a message when a javascript error occurs in display output var msg = "Javascript error adding output!" if ( element === undefined ) return; - element.append( - $('
').html(msg + "
" + - err.toString() + - '
See your browser Javascript console for more details.' - ).addClass('js-error') - ); + element + .append($('
').text(msg).addClass('js-error')) + .append($('
').text(err.toString()).addClass('js-error')) + .append($('
').text('See your browser Javascript console for more details.').addClass('js-error')); }; OutputArea.prototype._safe_append = function (toinsert) { @@ -447,6 +446,8 @@ var IPython = (function (IPython) { var pre = this.element.find('div.'+subclass).last().find('pre'); var html = utils.fixCarriageReturn( pre.html() + utils.fixConsole(text)); + // The only user content injected with with this HTML call is + // escaped by the fixConsole() method. pre.html(html); return; } @@ -548,6 +549,8 @@ var IPython = (function (IPython) { if (extra_class){ toinsert.addClass(extra_class); } + // The only user content injected with with this HTML call is + // escaped by the fixConsole() method. toinsert.append($("
").html(data));
         element.append(toinsert);
         return toinsert;
diff --git a/IPython/html/static/notebook/js/pager.js b/IPython/html/static/notebook/js/pager.js
index 697769e11..5f1e97636 100644
--- a/IPython/html/static/notebook/js/pager.js
+++ b/IPython/html/static/notebook/js/pager.js
@@ -164,6 +164,8 @@ var IPython = (function (IPython) {
     }
 
     Pager.prototype.append_text = function (text) {
+        // The only user content injected with with this HTML call is escaped by
+        // the fixConsole() method.
         this.pager_element.find(".container").append($('
').html(utils.fixCarriageReturn(utils.fixConsole(text))));
     };
 
diff --git a/IPython/html/static/notebook/js/textcell.js b/IPython/html/static/notebook/js/textcell.js
index b3493e7c9..b45e29a59 100644
--- a/IPython/html/static/notebook/js/textcell.js
+++ b/IPython/html/static/notebook/js/textcell.js
@@ -245,7 +245,7 @@ var IPython = (function (IPython) {
      * @method set_rendered
      */
     TextCell.prototype.set_rendered = function(text) {
-        this.element.find('div.text_cell_render').html(text);
+        this.element.find('div.text_cell_render').text(text);
     };
 
     /**
@@ -350,15 +350,20 @@ var IPython = (function (IPython) {
             math = text_and_math[1];
             var html = marked.parser(marked.lexer(text));
             html = $(IPython.mathjaxutils.replace_math(html, math));
-            // links in markdown cells should open in new tabs
+            // Links in markdown cells should open in new tabs.
             html.find("a[href]").not('[href^="#"]').attr("target", "_blank");
             try {
-                this.set_rendered(html);
+                // TODO: This HTML needs to be treated as potentially dangerous
+                // user input.
+                rendered.html(html);
             } catch (e) {
                 console.log("Error running Javascript in Markdown:");
                 console.log(e);
-                this.set_rendered($("
").addClass("js-error").html( - "Error rendering Markdown!
" + e.toString()) + rendered.empty(); + rendered.append( + $("
") + .append($("
").text('Error rendering Markdown!').addClass("js-error")) + .append($("
").text(e.toString()).addClass("js-error")) ); } this.element.find('div.text_cell_input').hide(); @@ -504,11 +509,6 @@ var IPython = (function (IPython) { }; - HeadingCell.prototype.set_rendered = function (html) { - this.element.find("div.text_cell_render").html(html); - }; - - HeadingCell.prototype.get_rendered = function () { var r = this.element.find("div.text_cell_render"); return r.children().first().html(); @@ -538,11 +538,13 @@ var IPython = (function (IPython) { .attr('href', '#' + hash) .text('¶') ); - - this.set_rendered(h); + // TODO: This HTML needs to be treated as potentially dangerous + // user input. + var rendered = this.element.find("div.text_cell_render"); + rendered.html(h); this.typeset(); this.element.find('div.text_cell_input').hide(); - this.element.find("div.text_cell_render").show(); + rendered.show(); }; return cont; diff --git a/IPython/html/static/notebook/js/tooltip.js b/IPython/html/static/notebook/js/tooltip.js index 39bc34b9b..c04d34b9d 100644 --- a/IPython/html/static/notebook/js/tooltip.js +++ b/IPython/html/static/notebook/js/tooltip.js @@ -369,6 +369,7 @@ var IPython = (function (IPython) { this._hidden = false; this.text.children().remove(); + // Any HTML within the docstring is escaped by the fixConsole() method. var pre = $('
').html(utils.fixConsole(docstring));
         if (defstring) {
             var defstring_html = $('
').html(utils.fixConsole(defstring));

From 6a224d131b4509a3ddc842b74bc2149438c0e3ac Mon Sep 17 00:00:00 2001
From: Jonathan Frederic 
Date: Fri, 21 Feb 2014 09:40:14 -0800
Subject: [PATCH 2/5] Move todos into set_rendered

---
 IPython/html/static/notebook/js/textcell.js | 30 +++++++++++----------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/IPython/html/static/notebook/js/textcell.js b/IPython/html/static/notebook/js/textcell.js
index b45e29a59..9bdf9e20f 100644
--- a/IPython/html/static/notebook/js/textcell.js
+++ b/IPython/html/static/notebook/js/textcell.js
@@ -245,7 +245,9 @@ var IPython = (function (IPython) {
      * @method set_rendered
      */
     TextCell.prototype.set_rendered = function(text) {
-        this.element.find('div.text_cell_render').text(text);
+        // TODO: This HTML needs to be treated as potentially dangerous
+        // user input.         
+        this.element.find('div.text_cell_render').html(text);
     };
 
     /**
@@ -353,17 +355,12 @@ var IPython = (function (IPython) {
             // Links in markdown cells should open in new tabs.
             html.find("a[href]").not('[href^="#"]').attr("target", "_blank");
             try {
-                // TODO: This HTML needs to be treated as potentially dangerous
-                // user input.
-                rendered.html(html);
+                this.set_rendered(html);
             } catch (e) {
                 console.log("Error running Javascript in Markdown:");
                 console.log(e);
-                rendered.empty();
-                rendered.append(
-                    $("
") - .append($("
").text('Error rendering Markdown!').addClass("js-error")) - .append($("
").text(e.toString()).addClass("js-error")) + this.set_rendered($("
").addClass("js-error").html( + "Error rendering Markdown!
" + e.toString()) ); } this.element.find('div.text_cell_input').hide(); @@ -509,6 +506,13 @@ var IPython = (function (IPython) { }; + HeadingCell.prototype.set_rendered = function (html) { + // TODO: This HTML needs to be treated as potentially dangerous + // user input. + this.element.find("div.text_cell_render").html(html); + }; + + HeadingCell.prototype.get_rendered = function () { var r = this.element.find("div.text_cell_render"); return r.children().first().html(); @@ -538,13 +542,11 @@ var IPython = (function (IPython) { .attr('href', '#' + hash) .text('¶') ); - // TODO: This HTML needs to be treated as potentially dangerous - // user input. - var rendered = this.element.find("div.text_cell_render"); - rendered.html(h); + + this.set_rendered(h); this.typeset(); this.element.find('div.text_cell_input').hide(); - rendered.show(); + this.element.find("div.text_cell_render").show(); }; return cont; From 407fc0a017a2379188eb8e0c3884ea0b14c0f183 Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Fri, 21 Feb 2014 09:42:14 -0800 Subject: [PATCH 3/5] s/with with/with --- IPython/html/static/notebook/js/outputarea.js | 4 ++-- IPython/html/static/notebook/js/pager.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/IPython/html/static/notebook/js/outputarea.js b/IPython/html/static/notebook/js/outputarea.js index 17b41437e..aceb0f788 100644 --- a/IPython/html/static/notebook/js/outputarea.js +++ b/IPython/html/static/notebook/js/outputarea.js @@ -446,7 +446,7 @@ var IPython = (function (IPython) { var pre = this.element.find('div.'+subclass).last().find('pre'); var html = utils.fixCarriageReturn( pre.html() + utils.fixConsole(text)); - // The only user content injected with with this HTML call is + // The only user content injected with this HTML call is // escaped by the fixConsole() method. pre.html(html); return; @@ -549,7 +549,7 @@ var IPython = (function (IPython) { if (extra_class){ toinsert.addClass(extra_class); } - // The only user content injected with with this HTML call is + // The only user content injected with this HTML call is // escaped by the fixConsole() method. toinsert.append($("
").html(data));
         element.append(toinsert);
diff --git a/IPython/html/static/notebook/js/pager.js b/IPython/html/static/notebook/js/pager.js
index 5f1e97636..aac9bfb21 100644
--- a/IPython/html/static/notebook/js/pager.js
+++ b/IPython/html/static/notebook/js/pager.js
@@ -164,7 +164,7 @@ var IPython = (function (IPython) {
     }
 
     Pager.prototype.append_text = function (text) {
-        // The only user content injected with with this HTML call is escaped by
+        // The only user content injected with this HTML call is escaped by
         // the fixConsole() method.
         this.pager_element.find(".container").append($('
').html(utils.fixCarriageReturn(utils.fixConsole(text))));
     };

From 946212d5fa9684887317678c6706b2af6ce4b946 Mon Sep 17 00:00:00 2001
From: Jonathan Frederic 
Date: Fri, 21 Feb 2014 10:05:07 -0800
Subject: [PATCH 4/5] Treat set_rendered as unsafe.

---
 IPython/html/static/notebook/js/textcell.js | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/IPython/html/static/notebook/js/textcell.js b/IPython/html/static/notebook/js/textcell.js
index 9bdf9e20f..cd94a0e39 100644
--- a/IPython/html/static/notebook/js/textcell.js
+++ b/IPython/html/static/notebook/js/textcell.js
@@ -245,8 +245,6 @@ var IPython = (function (IPython) {
      * @method set_rendered
      */
     TextCell.prototype.set_rendered = function(text) {
-        // TODO: This HTML needs to be treated as potentially dangerous
-        // user input.         
         this.element.find('div.text_cell_render').html(text);
     };
 
@@ -297,6 +295,8 @@ var IPython = (function (IPython) {
                 // make this value the starting point, so that we can only undo
                 // to this state, instead of a blank cell
                 this.code_mirror.clearHistory();
+                // TODO: This HTML needs to be treated as potentially dangerous
+                // user input and should be handled before set_rendered.         
                 this.set_rendered(data.rendered || '');
                 this.rendered = false;
                 this.render();
@@ -355,12 +355,17 @@ var IPython = (function (IPython) {
             // Links in markdown cells should open in new tabs.
             html.find("a[href]").not('[href^="#"]').attr("target", "_blank");
             try {
+                // TODO: This HTML needs to be treated as potentially dangerous
+                // user input and should be handled before set_rendered.         
                 this.set_rendered(html);
             } catch (e) {
                 console.log("Error running Javascript in Markdown:");
                 console.log(e);
-                this.set_rendered($("
").addClass("js-error").html( - "Error rendering Markdown!
" + e.toString()) + rendered.empty(); + rendered.append( + $("
") + .append($("
").text('Error rendering Markdown!').addClass("js-error")) + .append($("
").text(e.toString()).addClass("js-error")) ); } this.element.find('div.text_cell_input').hide(); @@ -507,8 +512,6 @@ var IPython = (function (IPython) { HeadingCell.prototype.set_rendered = function (html) { - // TODO: This HTML needs to be treated as potentially dangerous - // user input. this.element.find("div.text_cell_render").html(html); }; @@ -542,7 +545,8 @@ var IPython = (function (IPython) { .attr('href', '#' + hash) .text('¶') ); - + // TODO: This HTML needs to be treated as potentially dangerous + // user input and should be handled before set_rendered. this.set_rendered(h); this.typeset(); this.element.find('div.text_cell_input').hide(); From ef3f61f9068eea2a9c11f6453b2df12f9bac903c Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Fri, 21 Feb 2014 10:31:29 -0800 Subject: [PATCH 5/5] Use set_rendered to set the error msg of the cell. --- IPython/html/static/notebook/js/textcell.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/IPython/html/static/notebook/js/textcell.js b/IPython/html/static/notebook/js/textcell.js index cd94a0e39..e9c943db2 100644 --- a/IPython/html/static/notebook/js/textcell.js +++ b/IPython/html/static/notebook/js/textcell.js @@ -361,11 +361,11 @@ var IPython = (function (IPython) { } catch (e) { console.log("Error running Javascript in Markdown:"); console.log(e); - rendered.empty(); - rendered.append( + this.set_rendered( $("
") .append($("
").text('Error rendering Markdown!').addClass("js-error")) .append($("
").text(e.toString()).addClass("js-error")) + .html() ); } this.element.find('div.text_cell_input').hide();