From 1e430f931e6b122cb84aa05f7e2506140ff2b325 Mon Sep 17 00:00:00 2001
From: Joel Chen <xchen@walmartlabs.com>
Date: Tue, 19 Jan 2021 17:28:01 -0800
Subject: [PATCH] [fix] subapp2 safe stringify props for client

---
 packages/xarc-subapp/src/browser/xarc-subapp-v2.ts    |  2 +-
 packages/xarc-subapp/src/node/init-v2.ts              |  6 +++---
 .../xarc-subapp/src/node/server-render-pipeline.ts    |  8 ++++++--
 packages/xarc-subapp/src/node/utils.ts                | 11 +++++++++++
 samples/subapp2-poc/src/demo3-static-props.ts         |  8 +++++++-
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/packages/xarc-subapp/src/browser/xarc-subapp-v2.ts b/packages/xarc-subapp/src/browser/xarc-subapp-v2.ts
index b0f0553a9..ffe90bc3f 100644
--- a/packages/xarc-subapp/src/browser/xarc-subapp-v2.ts
+++ b/packages/xarc-subapp/src/browser/xarc-subapp-v2.ts
@@ -182,7 +182,7 @@ export function xarcV2Client(
         return {};
       } else {
         try {
-          return JSON.parse(element.innerHTML);
+          return JSON.parse(element.innerHTML.replace(/&lt;(\/?)script>/g, "<$1script>"));
         } catch (err) {
           console.error(msg, id, "- parse");
           return {};
diff --git a/packages/xarc-subapp/src/node/init-v2.ts b/packages/xarc-subapp/src/node/init-v2.ts
index 9b0e1ddc3..caf66d7cb 100644
--- a/packages/xarc-subapp/src/node/init-v2.ts
+++ b/packages/xarc-subapp/src/node/init-v2.ts
@@ -1,11 +1,11 @@
 /* eslint-disable no-console, max-statements, global-require, @typescript-eslint/no-var-requires */
 
 import Path from "path";
-import { generateNonce, loadCdnMap, mapCdn, wrapStringFragment, urlJoin } from "./utils";
+import { loadCdnMap, mapCdn, wrapStringFragment, urlJoin } from "./utils";
 import { WebpackStats } from "./webpack-stats";
 import Crypto from "crypto";
 import { AssetPathMap, InitProps } from "./types";
-import { SSR_PIPELINES } from "./utils";
+import { SSR_PIPELINES, safeStringifyJson } from "./utils";
 
 /**
  * Initialize all the up front code required for running subapps in the browser.
@@ -56,7 +56,7 @@ export function initSubApp(setupContext: any, setupToken: Partial<{ props: InitP
   if (cdnMapData) {
     const cdnMapJsonId = `cdn-map-${Crypto.randomBytes(8).toString("base64")}`;
     cdnAsJsonScript = `<script{{SCRIPT_NONCE}} type="application/json" id="${cdnMapJsonId}">
-${JSON.stringify(cdnMapData)}
+${safeStringifyJson(cdnMapData)}
 </script>
 `;
     cdnUpdateScript = `window.xarcV2.cdnUpdate({md:window.xarcV2.dyn("${cdnMapJsonId}")})
diff --git a/packages/xarc-subapp/src/node/server-render-pipeline.ts b/packages/xarc-subapp/src/node/server-render-pipeline.ts
index e6daef52c..0b9bbe0e2 100644
--- a/packages/xarc-subapp/src/node/server-render-pipeline.ts
+++ b/packages/xarc-subapp/src/node/server-render-pipeline.ts
@@ -3,6 +3,7 @@ import _ from "lodash";
 import { SubAppRenderPipeline } from "../subapp/subapp-render-pipeline";
 import { SubAppSSRData, SubAppFeatureResult, LoadSubAppOptions } from "../subapp/types";
 import { ServerFrameworkLib } from "./types";
+import { safeStringifyJson } from "./utils";
 // global name to store client subapp runtime, ie: window.xarcV1
 // V1: version 1.
 const xarc = "window.xarcV2";
@@ -94,16 +95,19 @@ export class SubAppServerRenderPipeline implements SubAppRenderPipeline {
       const dataId = `${name}-initial-state-${Date.now()}-${++SubAppServerRenderPipeline.INITIAL_STATE_TAG_ID}`;
       initialStateData = `
 <script${scriptNonceAttr} type="application/json" id="${dataId}">
-${JSON.stringify(ssrProps)}
+${safeStringifyJson(ssrProps)}
 </script>`;
       initialStateScript = `${xarc}.dyn("${dataId}")`;
     }
 
+    // about using safeStringifyJson here: We don't expect user to write their own code
+    // with <script> or </script> in their options, but if they do, it's their problem,
+    // but we at least avoid code blowing up due to that.
     this.outputSpot.add(
       `<!-- time: ${this.startTime} -->
 ${ssrContent}${initialStateData}
 <!-- time: ${now} diff: ${now - this.startTime} -->
-<script${scriptNonceAttr}>${xarc}.startSubAppOnLoad(${JSON.stringify(this.options)},
+<script${scriptNonceAttr}>${xarc}.startSubAppOnLoad(${safeStringifyJson(this.options)},
 {getInitialState:function(){return ${initialStateScript};}});</script>
 `
     );
diff --git a/packages/xarc-subapp/src/node/utils.ts b/packages/xarc-subapp/src/node/utils.ts
index 30f43fad4..1039aeac8 100644
--- a/packages/xarc-subapp/src/node/utils.ts
+++ b/packages/xarc-subapp/src/node/utils.ts
@@ -101,3 +101,14 @@ export function urlJoin(baseUrl: string, ...pathParts: string[]) {
 }
 
 export const SSR_PIPELINES = Symbol("subapp-ssr-pipelines");
+
+/**
+ * Stringify a JSON object and replace some tags to avoid XSS:
+ *  - `<script>` => `&lt;script>`
+ *  - `</script>` => `&lt;/script>`
+ * @param obj - object to stringify
+ * @returns JSON string of object
+ */
+export function safeStringifyJson(obj) {
+  return JSON.stringify(obj).replace(/<(\/?)script>/g, "&lt;$1script>");
+}
diff --git a/samples/subapp2-poc/src/demo3-static-props.ts b/samples/subapp2-poc/src/demo3-static-props.ts
index eb7fc705a..a7aab124c 100644
--- a/samples/subapp2-poc/src/demo3-static-props.ts
+++ b/samples/subapp2-poc/src/demo3-static-props.ts
@@ -6,7 +6,13 @@ export const getStaticProps = async () => {
     setTimeout(resolve, delay);
   }).then(() => {
     return {
-      props: { message: "demo3 this is static props", delay }
+      props: {
+        message: "demo3 this is static props",
+        // this actually won't execute even if we didn't escape the <script> tags
+        // because this sample enables CSP nonce
+        xss: "</script><script>alert('oops, xss')</script>",
+        delay
+      }
     };
   });
 };