Attempt to fix potential content page console.log code execution/inje…

…ction
joelpurra · May 2, 2017 · 23e528d · 23e528d
1 parent 402f3be
commit 23e528d
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 66 deletions.
diff --git a/src/shared/content-logger.js b/src/shared/content-logger.js
@@ -21,7 +21,6 @@ along with Talkie.  If not, see <https://www.gnu.org/licenses/>.
 import {
     logInfo,
     logWarn,
-    variableToSafeConsoleLogString,
 } from "../shared/log";
 
 import {
@@ -37,17 +36,6 @@ export default class ContentLogger {
         this.executeLogToPageWithColorCode = "(function(){ try { console.log(%a); } catch (error) { console.error('Talkie', 'logToPageWithColor', error); } }());";
     }
 
-    _variableToSafeConsoleLogString(value) {
-        const str = variableToSafeConsoleLogString(value);
-
-        const friendlyStr = str
-            // NOTE: escaping for passing the string to be executed in the page content context.
-            .replace(/\\/g, "\\\\")
-            .replace(/\\\\"/g, "\\\"");
-
-        return friendlyStr;
-    }
-
     logToPage(...args) {
         return promiseTry(
             () => {
@@ -56,10 +44,9 @@ export default class ContentLogger {
                 const logValues = [
                     now,
                     this.configuration.extensionShortName,
-                    ...args.map((arg) => this._variableToSafeConsoleLogString(arg)),
+                    ...args,
                 ]
-                    // NOTE: quote each console.log() argument.
-                    .map((arg) => `"${arg}"`)
+                    .map((arg) => JSON.stringify(arg))
                     .join(", ");
 
                 const code = this.executeLogToPageCode.replace("%a", logValues);
@@ -85,13 +72,17 @@ export default class ContentLogger {
                 const now = new Date().toISOString();
 
                 // NOTE: create one long console.log() string argument, then add the color argument second.
-                const logValues = "\"" + [
+                const logValuesArrayAsString = [
                     now,
                     this.configuration.extensionShortName,
                     "%c",
-                    ...args.map((arg) => this._variableToSafeConsoleLogString(arg)),
+                    ...args,
                 ]
-                    .join(" ") + " " + "\", \"background: #007F41; color: #FFFFFF; padding: 0.3em;\"";
+                    .join(" ");
+
+                const logValues = JSON.stringify(logValuesArrayAsString)
+                    + ", "
+                    + JSON.stringify("background: #007F41; color: #FFFFFF; padding: 0.3em;");
 
                 const code = this.executeLogToPageWithColorCode.replace("%a", logValues);
 

diff --git a/src/shared/log.js b/src/shared/log.js
@@ -20,52 +20,6 @@ along with Talkie.  If not, see <https://www.gnu.org/licenses/>.
 
 const extensionShortName = browser.i18n.getMessage("extensionShortName");
 
-export const _variableToSafeConsoleLogStringReplacer = (/* eslint-disable no-unused-vars */key/* eslint-enable no-unused-vars */, value) => {
-    // NOTE: want to display if undefined was passed.
-    if (typeof value === "undefined") {
-        return "undefined";
-    }
-
-    // NOTE: want to display if a function was passed.
-    if (typeof value === "function") {
-        return "function";
-    }
-
-    // NOTE: should take care of all other cases best as it can.
-    // https://github.com/joelpurra/talkie/issues/6
-    return value;
-};
-
-export const variableToSafeConsoleLogString = (value) => {
-    // NOTE: want to display if undefined was passed.
-    if (typeof value === "undefined") {
-        return "undefined";
-    }
-
-    // NOTE: want to display if a function was passed.
-    if (typeof value === "function") {
-        return "function";
-    }
-
-    // NOTE: should take care of all other cases best as it can.
-    // https://github.com/joelpurra/talkie/issues/6
-    const json = JSON.stringify(value, _variableToSafeConsoleLogStringReplacer);
-
-    const friendlyJson = json
-        // NOTE: don't double-quote standalone strings, just to make output prettier.
-        .replace(/^"/, "")
-        .replace(/"$/, "")
-        // NOTE: line separator and paragraph separator encoding.
-        // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Issue_with_plain_JSON.stringify_for_use_as_JavaScript
-        .replace(/\u2028/g, "\\u2028")
-        .replace(/\u2029/g, "\\u2029")
-        // NOTE: there should be no newlines in the JSON, so this might be unncessary.
-        .replace(/\n/g, "\\\\n")
-        .replace(/\r/g, "\\\\r");
-
-    return friendlyJson;
-};
-
 // https://stackoverflow.com/questions/12830649/check-if-chrome-extension-installed-in-unpacked-mode
 // http://stackoverflow.com/a/20227975
 const isDevMode = () => !("update_url" in browser.runtime.getManifest());
@@ -146,8 +100,7 @@ const generateLogger = (loggingLevelName, consoleFunctioName) => {
             // NOTE: for chrome command line console debugging.
             // NOTE: has to be an array.
             loggingArgs = [
-                loggingArgs.map((loggingArg) => variableToSafeConsoleLogString(loggingArg))
-                    .join(" "),
+                JSON.stringify(loggingArgs),
             ];
         }