[fix] subapp2 safe stringify props for client

electrode-io · Jan 20, 2021 · 1e430f9 · 1e430f9
1 parent f71798d
commit 1e430f9
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
+      }
     };
   });
 };