Bug 1708356 - [devtools] Only strip true XSSI prevention sequences while formatting JSON. r=bomsy

This fix makes sure that only valid XSSI prevention sequences are removed
from JSON payloads. Before, any string prepending the JSON payload was
removed which meant malformed JSON could still be displayed in properties
view as if it was valid which confused users.

Differential Revision: https://phabricator.services.mozilla.com/D115442

devtools/client/netmonitor/src/utils/request-utils.js

@@ -625,14 +625,20 @@ function isBase64(payload) {
 
 /**
  * Checks if the payload is of JSON type.
- * This function also handles JSON with XSSI-escaping characters by skipping them.
+ * This function also handles JSON with XSSI-escaping characters by stripping them
+ * and returning the stripped chars in the strippedChars property
  * This function also handles Base64 encoded JSON.
+ * @returns {Object} shape:
+ *  {Object} json: parsed JSON object
+ *  {Error} error: JSON parsing error
+ *  {string} strippedChars: XSSI stripped chars removed from JSON payload
  */
 function parseJSON(payloadUnclean) {
-  let json, error;
+  let json;
   const jsonpRegex = /^\s*([\w$]+)\s*\(\s*([^]*)\s*\)\s*;?\s*$/;
   const [, jsonpCallback, jsonp] = payloadUnclean.match(jsonpRegex) || [];
   if (jsonpCallback && jsonp) {
+    let error;
     try {
       json = parseJSON(jsonp).json;
     } catch (err) {
@@ -640,32 +646,9 @@ function parseJSON(payloadUnclean) {
     }
     return { json, error, jsonpCallback };
   }
-  // Start at the first likely JSON character,
-  // so that magic XSSI characters can be avoided
-  const firstSquare = payloadUnclean.indexOf("[");
-  const firstCurly = payloadUnclean.indexOf("{");
-  // This logic finds the first starting square or curly bracket.
-  // However, since Math.min will return -1 even if
-  // the other type of bracket was found and has an index,
-  // if one of the indexes is -1, the max value is returned
-  // (this value may also be -1, but that is checked for later on.)
-  const minFirst = Math.min(firstSquare, firstCurly);
-  let first;
-  if (minFirst === -1) {
-    first = Math.max(firstCurly, firstSquare);
-  } else {
-    first = minFirst;
-  }
-  let payload = "";
-  if (first !== -1) {
-    try {
-      payload = payloadUnclean.substring(first);
-    } catch (err) {
-      error = err;
-    }
-  } else {
-    payload = payloadUnclean;
-  }
+
+  let { payload, strippedChars, error } = removeXSSIString(payloadUnclean);
+
   try {
     json = JSON.parse(payload);
   } catch (err) {
@@ -690,6 +673,46 @@ function parseJSON(payloadUnclean) {
   return {
     json,
     error,
+    strippedChars,
+  };
+}
+
+/**
+ * Removes XSSI prevention sequences from JSON payloads
+ * @param {string} payloadUnclean: JSON payload that may or may have a
+ *                                 XSSI prevention sequence
+ * @returns {Object} Shape:
+ *   {string} payload: the JSON witht the XSSI prevention sequence removed
+ *   {string} strippedChars: XSSI string that was removed, null if no XSSI
+ *                           prevention sequence was found
+ *   {Error} error: error attempting to strip XSSI prevention sequence
+ */
+function removeXSSIString(payloadUnclean) {
+  // Regex that finds the XSSI protection sequences )]}'\n for(;;); and while(1);
+  const xssiRegex = /(^\)\]\}',?\n)|(^for ?\(;;\);?)|(^while ?\(1\);?)/;
+  let payload, strippedChars, error;
+  const xssiRegexMatch = payloadUnclean.match(xssiRegex);
+
+  // Remove XSSI string if there was one found
+  if (xssiRegexMatch?.length > 0) {
+    const xssiLen = xssiRegexMatch[0].length;
+    try {
+      // substring the payload by the length of the XSSI match to remove it
+      // and save the match to report
+      payload = payloadUnclean.substring(xssiLen);
+      strippedChars = xssiRegexMatch[0];
+    } catch (err) {
+      error = err;
+      payload = payloadUnclean;
+    }
+  } else {
+    // if there was no XSSI match just return the raw payload
+    payload = payloadUnclean;
+  }
+  return {
+    payload,
+    strippedChars,
+    error,
   };
 }
 

devtools/client/netmonitor/test/browser.ini

@@ -28,6 +28,7 @@ support-files =
   html_json-long-test-page.html
   html_json-malformed-test-page.html
   html_json-text-mime-test-page.html
+  html_json-xssi-protection.html
   html_jsonp-test-page.html
   html_maps-test-page.html
   html_navigate-test-page.html
@@ -195,6 +196,7 @@ skip-if = verify # Bug 1607678
 [browser_net_json-nogrip.js]
 [browser_net_json_custom_mime.js]
 [browser_net_json_text_mime.js]
+[browser_net_json-xssi-protection.js]
 [browser_net_jsonp.js]
 [browser_net_large-response.js]
 [browser_net_leak_on_tab_close.js]

devtools/client/netmonitor/test/browser_net_json-xssi-protection.js

@@ -0,0 +1,72 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Tests if JSON responses and requests with XSSI protection sequences
+ * are handled correctly.
+ */
+
+add_task(async function() {
+  const { tab, monitor } = await initNetMonitor(JSON_XSSI_PROTECTION_URL, {
+    requestCount: 1,
+  });
+  info("Starting test... ");
+
+  const { document, store, windowRequire } = monitor.panelWin;
+  const Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
+
+  store.dispatch(Actions.batchEnable(false));
+
+  // Execute requests.
+  await performRequests(monitor, tab, 1);
+
+  let wait = waitForDOM(document, "#response-panel .data-header");
+  const waitForPropsView = waitForDOM(
+    document,
+    "#response-panel .properties-view",
+    1
+  );
+
+  store.dispatch(Actions.toggleNetworkDetails());
+  info("Opening response panel");
+  clickOnSidebarTab(document, "response");
+
+  await Promise.all([wait, waitForPropsView]);
+
+  const tabpanel = document.querySelector("#response-panel");
+  is(
+    tabpanel.querySelectorAll(".treeRow").length,
+    1,
+    "There should be 1 json property displayed in the response."
+  );
+
+  const labels = tabpanel.querySelectorAll("tr .treeLabelCell .treeLabel");
+  const values = tabpanel.querySelectorAll("tr .treeValueCell .objectBox");
+  info("Checking content of displayed json response");
+  is(labels[0].textContent, "greeting", "The first key should be correct");
+  is(
+    values[0].textContent,
+    `"Hello good XSSI protection"`,
+    "The first property should be correct"
+  );
+
+  info("switching to raw view");
+  wait = waitForDOM(document, "#response-panel .CodeMirror-code");
+  const rawResponseToggle = tabpanel.querySelector("#raw-response-checkbox");
+  clickElement(rawResponseToggle, monitor);
+  await wait;
+
+  info("making sure XSSI protection is in raw view");
+  const codeLines = document.querySelector("#response-panel .CodeMirror-code");
+  const firstLine = codeLines.firstChild;
+  const firstLineText = firstLine.querySelector("pre.CodeMirror-line span");
+  is(
+    firstLineText.textContent,
+    ")]}'",
+    "XSSI protection sequence should be visibly in raw view"
+  );
+
+  await teardown(monitor);
+});

devtools/client/netmonitor/test/head.js

@@ -100,6 +100,7 @@ const JSON_TEXT_MIME_URL = EXAMPLE_URL + "html_json-text-mime-test-page.html";
 const JSON_B64_URL = EXAMPLE_URL + "html_json-b64.html";
 const JSON_BASIC_URL = EXAMPLE_URL + "html_json-basic.html";
 const JSON_EMPTY_URL = EXAMPLE_URL + "html_json-empty.html";
+const JSON_XSSI_PROTECTION_URL = EXAMPLE_URL + "html_json-xssi-protection.html";
 const FONTS_URL = EXAMPLE_URL + "html_fonts-test-page.html";
 const SORTING_URL = EXAMPLE_URL + "html_sorting-test-page.html";
 const FILTERING_URL = EXAMPLE_URL + "html_filter-test-page.html";

devtools/client/netmonitor/test/html_json-xssi-protection.html

@@ -0,0 +1,42 @@
+
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+<!doctype html>
+
+<html>
+  <head>
+    <meta charset="utf-8"/>
+    <meta http-equiv="Cache-Control" content="no-cache, no-store, must-revalidate" />
+    <meta http-equiv="Pragma" content="no-cache" />
+    <meta http-equiv="Expires" content="0" />
+    <title>Network Monitor test page</title>
+  </head>
+
+  <body>
+    <p>JSON XSSI protection test</p>
+
+    <script type="text/javascript">
+      /* exported performRequests */
+      "use strict";
+
+      function get(address) {
+        return new Promise (resolve => {
+          const xhr = new XMLHttpRequest();
+          xhr.open("GET", address, true);
+
+          xhr.onreadystatechange = function() {
+            if (this.readyState == this.DONE) {
+              resolve();
+            }
+          };
+          xhr.send();
+        });
+      }
+
+      async function performRequests() {
+        await get("sjs_content-type-test-server.sjs?fmt=json-valid-xssi-protection");
+      }
+    </script>
+  </body>
+
+</html>

devtools/client/netmonitor/test/sjs_content-type-test-server.sjs

@@ -276,6 +276,15 @@ function handleRequest(request, response) {
           response.finish();
           break;
         }
+        case "json-valid-xssi-protection": {
+          response.setStatusLine(request.httpVersion, status, "OK");
+          response.setHeader("Content-Type", "text/json; charset=utf-8", false);
+          setCacheHeaders();
+          response.write(')]}\'\n{"greeting": "Hello good XSSI protection"}');
+          response.finish();
+          break;
+        }
+
         case "font": {
           response.setStatusLine(request.httpVersion, status, "OK");
           response.setHeader("Content-Type", "font/woff", false);

devtools/client/netmonitor/test/xpcshell/test_request-utils-parseJSON.js

@@ -0,0 +1,78 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test devtools/client/netmonitor/src/utils/request-utils.js function
+// |parseJSON| ensure that it correctly handles plain JSON, Base 64
+// encoded JSON, and JSON that has XSSI protection prepended to it
+
+"use strict";
+
+const { require } = ChromeUtils.import(
+  "resource://devtools/shared/loader/Loader.jsm"
+);
+const {
+  parseJSON,
+} = require("devtools/client/netmonitor/src/utils/request-utils");
+
+function run_test() {
+  const validJSON = '{"item":{"subitem":true},"seconditem":"bar"}';
+  const base64JSON = btoa(validJSON);
+  const parsedJSON = { item: { subitem: true }, seconditem: "bar" };
+  const googleStyleXSSI = ")]}'\n";
+  const facebookStyleXSSI = "for (;;);";
+  const notRealXSSIPrevention = "sdgijsdjg";
+  const while1XSSIPrevention = "while(1)";
+
+  const parsedValidJSON = parseJSON(validJSON);
+  info(JSON.stringify(parsedValidJSON));
+  ok(
+    parsedValidJSON.json.item.subitem == parsedJSON.item.subitem &&
+      parsedValidJSON.json.seconditem == parsedJSON.seconditem,
+    "plain JSON is parsed correctly"
+  );
+
+  const parsedBase64JSON = parseJSON(base64JSON);
+  ok(
+    parsedBase64JSON.json.item.subitem === parsedJSON.item.subitem &&
+      parsedBase64JSON.json.seconditem === parsedJSON.seconditem,
+    "base64 encoded JSON is parsed correctly"
+  );
+
+  const parsedGoogleStyleXSSIJSON = parseJSON(googleStyleXSSI + validJSON);
+  ok(
+    parsedGoogleStyleXSSIJSON.strippedChars === googleStyleXSSI &&
+      parsedGoogleStyleXSSIJSON.error === void 0 &&
+      parsedGoogleStyleXSSIJSON.json.item.subitem === parsedJSON.item.subitem &&
+      parsedGoogleStyleXSSIJSON.json.seconditem === parsedJSON.seconditem,
+    "Google style XSSI sequence correctly removed and returned"
+  );
+
+  const parsedFacebookStyleXSSIJSON = parseJSON(facebookStyleXSSI + validJSON);
+  ok(
+    parsedFacebookStyleXSSIJSON.strippedChars === facebookStyleXSSI &&
+      parsedFacebookStyleXSSIJSON.error === void 0 &&
+      parsedFacebookStyleXSSIJSON.json.item.subitem ===
+        parsedJSON.item.subitem &&
+      parsedFacebookStyleXSSIJSON.json.seconditem === parsedJSON.seconditem,
+    "Facebook style XSSI sequence correctly removed and returned"
+  );
+
+  const parsedWhileXSSIJSON = parseJSON(while1XSSIPrevention + validJSON);
+  ok(
+    parsedWhileXSSIJSON.strippedChars === while1XSSIPrevention &&
+      parsedWhileXSSIJSON.error === void 0 &&
+      parsedWhileXSSIJSON.json.item.subitem === parsedJSON.item.subitem &&
+      parsedWhileXSSIJSON.json.seconditem === parsedJSON.seconditem,
+    "While XSSI sequence correctly removed and returned"
+  );
+  const parsedInvalidJson = parseJSON(notRealXSSIPrevention + validJSON);
+  ok(
+    !parsedInvalidJson.json && !parsedInvalidJson.strippedChars,
+    "Parsed invalid JSON does not return a data object or strippedChars"
+  );
+  equal(
+    parsedInvalidJson.error.name,
+    "SyntaxError",
+    "Parsing invalid JSON yeilds a SyntaxError"
+  );
+}

devtools/client/netmonitor/test/xpcshell/xpcshell.ini

@@ -7,3 +7,4 @@ skip-if = toolkit == 'android'
 [test_doc-utils.js]
 [test_request-utils-fetchNetworkUpdatePacket.js]
 [test_request-utils-js-getFormattedProtocol.js]
+[test_request-utils-parseJSON.js]