CVE-2017-18635: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: An XSS vulnerability was discovered in noVNC before 0.6.2 in which the remote VNC server could inject arbitrary HTML into the noVNC web page via the messages propagated to the status field, such as the VNC server name.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2019-09-25T23:15Z
Repository: https://github.com/novnc/noVNC
Language: HTML
Files changed: app/ui.js, tests/input.html, tests/vnc_perf.html, tests/vnc_playback.html, vnc_auto.html
Commit date: 2017-01-12 14:47:36 -0500
Commit message: Use textContent instead of innerHTML

Previously, setting `innerHTML` was used to display the statuses.  These
could include content communicated from the remote VNC server, allowing
the remove VNC server to inject HTML into the noVNC page.

This commit switches all uses of `innerHTML` to use `textContent`, which
is not vulnerable to the HTML injection.

Diff: diff --git a/app/ui.js b/app/ui.js
index 0e789c0..8056078 100644
--- a/app/ui.js
+++ b/app/ui.js
@@ -46,11 +46,11 @@ var UI;
                 msg += "</div>";
             }
 
             document.getElementById('noVNC_fallback_error')
                 .classList.add("noVNC_open");
-            document.getElementById('noVNC_fallback_errormsg').innerHTML = msg;
+            document.getElementById('noVNC_fallback_errormsg').textContent = msg;
         } catch (exc) {
             document.write("noVNC encountered an error.");
         }
         // Don't return true since this would prevent the error
         // from being printed to the browser console.
@@ -414,11 +414,11 @@ var UI;
             document.documentElement.classList.remove("noVNC_connected");
             document.documentElement.classList.remove("noVNC_disconnecting");
 
             switch (state) {
                 case 'connecting':
-                    document.getElementById("noVNC_transition_text").innerHTML = _("Connecting...");
+                    document.getElementById("noVNC_transition_text").textContent = _("Connecting...");
                     document.documentElement.classList.add("noVNC_connecting");
                     break;
                 case 'connected':
                     UI.connected = true;
                     document.documentElement.classList.add("noVNC_connected");
@@ -429,11 +429,11 @@ var UI;
                     }
                     UI.showStatus(msg);
                     break;
                 case 'disconnecting':
                     UI.connected = false;
-                    document.getElementById("noVNC_transition_text").innerHTML = _("Disconnecting...");
+                    document.getElementById("noVNC_transition_text").textContent = _("Disconnecting...");
                     document.documentElement.classList.add("noVNC_disconnecting");
                     break;
                 case 'disconnected':
                     UI.showStatus(_("Disconnected"));
                     break;
@@ -529,11 +529,11 @@ var UI;
                 default:
                     statusElem.classList.add("noVNC_status_normal");
                     break;
             }
 
-            statusElem.innerHTML = text;
+            statusElem.textContent = text;
             statusElem.classList.add("noVNC_open");
 
             // If no time was specified, show the status for 1.5 seconds
             if (typeof time === 'undefined') {
                 time = 1500;
diff --git a/tests/input.html b/tests/input.html
index 437d6f3..0938a4a 100644
--- a/tests/input.html
+++ b/tests/input.html
@@ -43,11 +43,11 @@
         }
 
         function message(str) {
             console.log(str);
             cell = document.getElementById('messages');
-            cell.innerHTML += msg_cnt + ": " + str + newline;
+            cell.textContent += msg_cnt + ": " + str + newline;
             cell.scrollTop = cell.scrollHeight;
             msg_cnt++;
         }
 
         function mouseButton(x, y, down, bmask) {
diff --git a/tests/vnc_perf.html b/tests/vnc_perf.html
index c3e6a11..ce97ca4 100644
--- a/tests/vnc_perf.html
+++ b/tests/vnc_perf.html
@@ -63,11 +63,11 @@
             passCur, passTot, passMin, passMax;
 
         function msg(str) {
             console.log(str);
             var cell = document.getElementById('messages');
-            cell.innerHTML += str + "\n";
+            cell.textContent += str + "\n";
             cell.scrollTop = cell.scrollHeight;
         }
         function dbgmsg(str) {
             if (Util.get_logging() === 'debug') {
                 msg(str);
@@ -83,11 +83,11 @@
                 test_state = 'failed';
             }
         }
 
         notification = function (rfb, mesg, level, options) {
-            document.getElementById('VNC_status').innerHTML = mesg;
+            document.getElementById('VNC_status').textContent = mesg;
         }
 
         function do_test() {
             document.getElementById('startButton').value = "Running";
             document.getElementById('startButton').disabled = true;
diff --git a/tests/vnc_playback.html b/tests/vnc_playback.html
index 510ad06..65b735e 100644
--- a/tests/vnc_playback.html
+++ b/tests/vnc_playback.html
@@ -47,11 +47,11 @@
         var fname, start_time;
 
         function message(str) {
             console.log(str);
             var cell = document.getElementById('messages');
-            cell.innerHTML += str + "\n";
+            cell.textContent += str + "\n";
             cell.scrollTop = cell.scrollHeight;
         }
 
         fname = WebUtil.getQueryVar('data', null);
         if (fname) {
@@ -74,11 +74,11 @@
                 test_state = 'failed';
             }
         }
 
         notification = function (rfb, mesg, level, options) {
-            document.getElementById('VNC_status').innerHTML = mesg;
+            document.getElementById('VNC_status').textContent = mesg;
         }
 
         function start() {
             document.getElementById('startButton').value = "Running";
             document.getElementById('startButton').disabled = true;
diff --git a/vnc_auto.html b/vnc_auto.html
index e86ae5d..e4fc467 100644
--- a/vnc_auto.html
+++ b/vnc_auto.html
@@ -109,14 +109,18 @@
                 msg = 'Password Required: ';
             }
             var html;
             html = '<form onsubmit="return setPassword();"';
             html += '  style="margin-bottom: 0px">';
-            html += msg;
+            html += '<label></label>'
             html += '<input type=password size=10 id="password_input" class="noVNC_status">';
             html += '<\/form>';
-            status(html, "warn");
+
+            // bypass status() because it sets text content
+            document.getElementById('noVNC_status_bar').setAttribute("class", "noVNC_status_warn");
+            document.getElementById('noVNC_status').innerHTML = html;
+            document.getElementById('noVNC_status').querySelector('label').textContent = msg;
         }
         function setPassword() {
             rfb.sendPassword(document.getElementById('password_input').value);
             return false;
         }
@@ -144,11 +148,11 @@
                     break;
                 default:
                     level = "warn";
             }
             document.getElementById('noVNC_status_bar').setAttribute("class", "noVNC_status_" + level);
-            document.getElementById('noVNC_status').innerHTML = text;
+            document.getElementById('noVNC_status').textContent = text;
         }
         function updateState(rfb, state, oldstate) {
             var cad = document.getElementById('sendCtrlAltDelButton');
             switch (state) {
                 case 'connecting':

Vulnerable code:             document.getElementById('noVNC_fallback_errormsg').innerHTML = msg;
                    document.getElementById("noVNC_transition_text").innerHTML = _("Connecting...");
                    document.getElementById("noVNC_transition_text").innerHTML = _("Disconnecting...");
            statusElem.innerHTML = text;
            cell.innerHTML += msg_cnt + ": " + str + newline;
            cell.innerHTML += str + "\n";
            document.getElementById('VNC_status').innerHTML = mesg;
            cell.innerHTML += str + "\n";
            document.getElementById('VNC_status').innerHTML = mesg;
            html += msg;
            status(html, "warn");
            document.getElementById('noVNC_status').innerHTML = text;
Fixed code:             document.getElementById('noVNC_fallback_errormsg').textContent = msg;
                    document.getElementById("noVNC_transition_text").textContent = _("Connecting...");
                    document.getElementById("noVNC_transition_text").textContent = _("Disconnecting...");
            statusElem.textContent = text;
            cell.textContent += msg_cnt + ": " + str + newline;
            cell.textContent += str + "\n";
            document.getElementById('VNC_status').textContent = mesg;
            cell.textContent += str + "\n";
            document.getElementById('VNC_status').textContent = mesg;
            html += '<label></label>'

            // bypass status() because it sets text content
            document.getElementById('noVNC_status_bar').setAttribute("class", "noVNC_status_warn");
            document.getElementById('noVNC_status').innerHTML = html;
            document.getElementById('noVNC_status').querySelector('label').textContent = msg;
            document.getElementById('noVNC_status').textContent = text;