CVE-2023-23630: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Eta is an embedded JS templating engine that works inside Node, Deno, and the browser. XSS attack - anyone using the Express API is impacted. The problem has been resolved. Users should upgrade to version 2.0.0. As a workaround, don't pass user supplied things directly to `res.render`. 

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: 2023-02-01T01:15Z
Repository: https://github.com/eta-dev/eta
Language: TypeScript
Files changed: deno_dist/file-handlers.ts, src/file-handlers.ts, test/file-handlers.spec.ts
Commit date: 2023-01-27 20:29:45 -0700
Commit message: Don't use data object for Eta configuration (#214)


Diff: diff --git a/deno_dist/file-handlers.ts b/deno_dist/file-handlers.ts
index 575d21e..168acea 100644
--- a/deno_dist/file-handlers.ts
+++ b/deno_dist/file-handlers.ts
@@ -2,11 +2,10 @@
 
 import EtaErr from "./err.ts";
 import compile from "./compile.ts";
 import { getConfig } from "./config.ts";
 import { getPath, readFile } from "./file-utils.ts";
-import { copyProps } from "./utils.ts";
 import { promiseImpl } from "./polyfills.ts";
 
 /* TYPES */
 
 import type {
@@ -17,14 +16,10 @@ import type {
 import type { TemplateFunction } from "./compile.ts";
 
 export type CallbackFn = (err: Error | null, str?: string) => void;
 
 interface DataObj {
-  /** Express.js settings may be stored here */
-  settings?: {
-    [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
-  };
   [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
 }
 
 interface PartialConfigWithFilename extends Partial<EtaConfig> {
   filename: string;
@@ -171,12 +166,11 @@ function includeFile(
  *
  * @param filepath Path to template file. If relative, specify `views` on the config object
  *
  * This can take two different function signatures:
  *
- * - `renderFile(filename, dataAndConfig, [cb])`
- *   - Eta will merge `dataAndConfig` into `eta.config`
+ * - `renderFile(filename, data, [cb])`
  * - `renderFile(filename, data, [config], [cb])`
  *
  * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result.
  *
  * **Examples**
@@ -187,11 +181,10 @@ function includeFile(
  *   console.log(rendered)
  * })
  *
  * let rendered = await eta.renderFile("./template.eta", data, {cache: true})
  *
- * let rendered = await eta.renderFile("./template", {...data, cache: true})
  * ```
  */
 
 function renderFile(
   filename: string,
@@ -222,18 +215,17 @@ function renderFile(
   cb?: CallbackFn,
 ): Promise<string> | void {
   /*
   Here we have some function overloading.
   Essentially, the first 2 arguments to renderFile should always be the filename and data
-  However, with Express, configuration options will be passed along with the data.
-  Thus, Express will call renderFile with (filename, dataAndOptions, cb)
-  And we want to also make (filename, data, options, cb) available
+  Express will call renderFile with (filename, data, cb)
+  We also want to make (filename, data, options, cb) available
   */
 
   let renderConfig: EtaConfigWithFilename;
   let callback: CallbackFn | undefined;
-  data = data || {}; // If data is undefined, we don't want accessing data.settings to error
+  data = data || {};
 
   // First, assign our callback function to `callback`
   // We can leave it undefined if neither parameter is a function;
   // Callbacks are optional
   if (typeof cb === "function") {
@@ -248,30 +240,12 @@ function renderFile(
   if (typeof config === "object") {
     renderConfig = getConfig(
       (config as PartialConfig) || {},
     ) as EtaConfigWithFilename;
   } else {
-    // Otherwise, get the config from the data object
-    // And then grab some config options from data.settings
-    // Which is where Express sometimes stores them
-    renderConfig = getConfig(data as PartialConfig) as EtaConfigWithFilename;
-    if (data.settings) {
-      // Pull a few things from known locations
-      if (data.settings.views) {
-        renderConfig.views = data.settings.views;
-      }
-      if (data.settings["view cache"]) {
-        renderConfig.cache = true;
-      }
-      // Undocumented after Express 2, but still usable, esp. for
-      // items that are unsafe to be passed along with data, like `root`
-      const viewOpts = data.settings["view options"];
-
-      if (viewOpts) {
-        copyProps(renderConfig, viewOpts);
-      }
-    }
+    // Otherwise, get the default config
+    renderConfig = getConfig({}) as EtaConfigWithFilename;
   }
 
   // Set the filename option on the template
   // This will first try to resolve the file path (see getPath for details)
   renderConfig.filename = getPath(filename, renderConfig);
@@ -284,12 +258,11 @@ function renderFile(
  *
  * @param filepath Path to template file. If relative, specify `views` on the config object
  *
  * This can take two different function signatures:
  *
- * - `renderFile(filename, dataAndConfig, [cb])`
- *   - Eta will merge `dataAndConfig` into `eta.config`
+ * - `renderFile(filename, data, [cb])`
  * - `renderFile(filename, data, [config], [cb])`
  *
  * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result.
  *
  * **Examples**
@@ -300,11 +273,10 @@ function renderFile(
  *   console.log(rendered)
  * })
  *
  * let rendered = await eta.renderFile("./template.eta", data, {cache: true})
  *
- * let rendered = await eta.renderFile("./template", {...data, cache: true})
  * ```
  */
 
 function renderFileAsync(
   filename: string,
diff --git a/src/file-handlers.ts b/src/file-handlers.ts
index 3a1c1b3..39534a6 100644
--- a/src/file-handlers.ts
+++ b/src/file-handlers.ts
@@ -2,25 +2,20 @@
 
 import EtaErr from "./err.js";
 import compile from "./compile.js";
 import { getConfig } from "./config.js";
 import { getPath, readFile } from "./file-utils.js";
-import { copyProps } from "./utils.js";
 import { promiseImpl } from "./polyfills.js";
 
 /* TYPES */
 
 import type { EtaConfig, PartialConfig, EtaConfigWithFilename } from "./config.js";
 import type { TemplateFunction } from "./compile.js";
 
 export type CallbackFn = (err: Error | null, str?: string) => void;
 
 interface DataObj {
-  /** Express.js settings may be stored here */
-  settings?: {
-    [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
-  };
   [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
 }
 
 interface PartialConfigWithFilename extends Partial<EtaConfig> {
   filename: string;
@@ -148,12 +143,11 @@ function includeFile(path: string, options: EtaConfig): [TemplateFunction, EtaCo
  *
  * @param filepath Path to template file. If relative, specify `views` on the config object
  *
  * This can take two different function signatures:
  *
- * - `renderFile(filename, dataAndConfig, [cb])`
- *   - Eta will merge `dataAndConfig` into `eta.config`
+ * - `renderFile(filename, data, [cb])`
  * - `renderFile(filename, data, [config], [cb])`
  *
  * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result.
  *
  * **Examples**
@@ -164,11 +158,10 @@ function includeFile(path: string, options: EtaConfig): [TemplateFunction, EtaCo
  *   console.log(rendered)
  * })
  *
  * let rendered = await eta.renderFile("./template.eta", data, {cache: true})
  *
- * let rendered = await eta.renderFile("./template", {...data, cache: true})
  * ```
  */
 
 function renderFile(filename: string, data: DataObj, config?: PartialConfig): Promise<string>;
 
@@ -190,18 +183,17 @@ function renderFile(
   cb?: CallbackFn
 ): Promise<string> | void {
   /*
   Here we have some function overloading.
   Essentially, the first 2 arguments to renderFile should always be the filename and data
-  However, with Express, configuration options will be passed along with the data.
-  Thus, Express will call renderFile with (filename, dataAndOptions, cb)
-  And we want to also make (filename, data, options, cb) available
+  Express will call renderFile with (filename, data, cb)
+  We also want to make (filename, data, options, cb) available
   */
 
   let renderConfig: EtaConfigWithFilename;
   let callback: CallbackFn | undefined;
-  data = data || {}; // If data is undefined, we don't want accessing data.settings to error
+  data = data || {};
 
   // First, assign our callback function to `callback`
   // We can leave it undefined if neither parameter is a function;
   // Callbacks are optional
   if (typeof cb === "function") {
@@ -214,30 +206,12 @@ function renderFile(
 
   // If there is a config object passed in explicitly, use it
   if (typeof config === "object") {
     renderConfig = getConfig((config as PartialConfig) || {}) as EtaConfigWithFilename;
   } else {
-    // Otherwise, get the config from the data object
-    // And then grab some config options from data.settings
-    // Which is where Express sometimes stores them
-    renderConfig = getConfig(data as PartialConfig) as EtaConfigWithFilename;
-    if (data.settings) {
-      // Pull a few things from known locations
-      if (data.settings.views) {
-        renderConfig.views = data.settings.views;
-      }
-      if (data.settings["view cache"]) {
-        renderConfig.cache = true;
-      }
-      // Undocumented after Express 2, but still usable, esp. for
-      // items that are unsafe to be passed along with data, like `root`
-      const viewOpts = data.settings["view options"];
-
-      if (viewOpts) {
-        copyProps(renderConfig, viewOpts);
-      }
-    }
+    // Otherwise, get the default config
+    renderConfig = getConfig({}) as EtaConfigWithFilename;
   }
 
   // Set the filename option on the template
   // This will first try to resolve the file path (see getPath for details)
   renderConfig.filename = getPath(filename, renderConfig);
@@ -250,12 +224,11 @@ function renderFile(
  *
  * @param filepath Path to template file. If relative, specify `views` on the config object
  *
  * This can take two different function signatures:
  *
- * - `renderFile(filename, dataAndConfig, [cb])`
- *   - Eta will merge `dataAndConfig` into `eta.config`
+ * - `renderFile(filename, data, [cb])`
  * - `renderFile(filename, data, [config], [cb])`
  *
  * Note that renderFile does not immediately return the rendered result. If you pass in a callback function, it will be called with `(err, res)`. Otherwise, `renderFile` will return a `Promise` that resolves to the render result.
  *
  * **Examples**
@@ -266,11 +239,10 @@ function renderFile(
  *   console.log(rendered)
  * })
  *
  * let rendered = await eta.renderFile("./template.eta", data, {cache: true})
  *
- * let rendered = await eta.renderFile("./template", {...data, cache: true})
  * ```
  */
 
 function renderFileAsync(filename: string, data: DataObj, config?: PartialConfig): Promise<string>;
 
diff --git a/test/file-handlers.spec.ts b/test/file-handlers.spec.ts
index 560d339..9aa68d9 100644
--- a/test/file-handlers.spec.ts
+++ b/test/file-handlers.spec.ts
@@ -65,32 +65,14 @@ describe("Simple renderFile tests", () => {
 
   it("uses cached version of a file", async () => {
     templates.define(fakeFilePath, compile("This template does not exist"));
 
     // renderFile should just look straight in the cache for the template
-    renderFile(fakeFilePath, { cache: true }, function (_err: Error | null, res?: string) {
+    renderFile(fakeFilePath, {}, { cache: true }, function (_err: Error | null, res?: string) {
       expect(res).toEqual("This template does not exist");
     });
   });
-
-  it("parses a simple template w/ settings from Express", async () => {
-    renderFile(
-      filePath,
-      {
-        name: "<p>Ben</p>",
-        cache: true,
-        settings: {
-          views: [path.join(__dirname, "templates"), path.join(__dirname, "othertemplates")],
-          "view cache": true,
-          "view options": { autoEscape: false },
-        },
-      },
-      function (_err: Error | null, res?: string) {
-        expect(res).toEqual("Hi <p>Ben</p>");
-      }
-    );
-  });
 });
 
 describe("File location tests", () => {
   it("locates a file with the views option", async () => {
     const res = await renderFile(
@@ -103,13 +85,13 @@ describe("File location tests", () => {
   });
 });
 
 describe("renderFile error tests", () => {
   it("render file with callback works on error", (done) => {
-    function cb(err: Error, _res?: string) {
+    function cb(err: Error | null, _res?: string) {
       expect(err).toBeTruthy();
-      expect(err.message).toMatch(
+      expect(err?.message).toMatch(
         buildRegEx(`
 var tR='',__l,__lP,include=E.include.bind(E),includeFile=E.includeFile.bind(E)
 function layout(p,d){__l=p;__lP=d}
 tR+='Hi '
 tR+=E.e(badSyntax(=!)
@@ -118,11 +100,11 @@ if(cb){cb(null,tR)} return tR
 `)
       );
       done();
     }
 
-    renderFile(errFilePath, { name: "Ada Lovelace", async: true }, cb);
+    renderFile(errFilePath, { name: "Ada Lovelace" }, { async: true }, cb);
   });
 
   test("throws with bad inner JS syntax using Promises", async () => {
     await expect(async () => {
       await renderFile(errFilePath, {});

Vulnerable code: import { copyProps } from "./utils.ts";
  /** Express.js settings may be stored here */
  settings?: {
    [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
  };
 * - `renderFile(filename, dataAndConfig, [cb])`
 *   - Eta will merge `dataAndConfig` into `eta.config`
 * let rendered = await eta.renderFile("./template", {...data, cache: true})
  However, with Express, configuration options will be passed along with the data.
  Thus, Express will call renderFile with (filename, dataAndOptions, cb)
  And we want to also make (filename, data, options, cb) available
  data = data || {}; // If data is undefined, we don't want accessing data.settings to error
    // Otherwise, get the config from the data object
    // And then grab some config options from data.settings
    // Which is where Express sometimes stores them
    renderConfig = getConfig(data as PartialConfig) as EtaConfigWithFilename;
    if (data.settings) {
      // Pull a few things from known locations
      if (data.settings.views) {
        renderConfig.views = data.settings.views;
      }
      if (data.settings["view cache"]) {
        renderConfig.cache = true;
      }
      // Undocumented after Express 2, but still usable, esp. for
      // items that are unsafe to be passed along with data, like `root`
      const viewOpts = data.settings["view options"];

      if (viewOpts) {
        copyProps(renderConfig, viewOpts);
      }
    }
 * - `renderFile(filename, dataAndConfig, [cb])`
 *   - Eta will merge `dataAndConfig` into `eta.config`
 * let rendered = await eta.renderFile("./template", {...data, cache: true})
import { copyProps } from "./utils.js";
  /** Express.js settings may be stored here */
  settings?: {
    [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
  };
 * - `renderFile(filename, dataAndConfig, [cb])`
 *   - Eta will merge `dataAndConfig` into `eta.config`
 * let rendered = await eta.renderFile("./template", {...data, cache: true})
  However, with Express, configuration options will be passed along with the data.
  Thus, Express will call renderFile with (filename, dataAndOptions, cb)
  And we want to also make (filename, data, options, cb) available
  data = data || {}; // If data is undefined, we don't want accessing data.settings to error
    // Otherwise, get the config from the data object
    // And then grab some config options from data.settings
    // Which is where Express sometimes stores them
    renderConfig = getConfig(data as PartialConfig) as EtaConfigWithFilename;
    if (data.settings) {
      // Pull a few things from known locations
      if (data.settings.views) {
        renderConfig.views = data.settings.views;
      }
      if (data.settings["view cache"]) {
        renderConfig.cache = true;
      }
      // Undocumented after Express 2, but still usable, esp. for
      // items that are unsafe to be passed along with data, like `root`
      const viewOpts = data.settings["view options"];

      if (viewOpts) {
        copyProps(renderConfig, viewOpts);
      }
    }
 * - `renderFile(filename, dataAndConfig, [cb])`
 *   - Eta will merge `dataAndConfig` into `eta.config`
 * let rendered = await eta.renderFile("./template", {...data, cache: true})
    renderFile(fakeFilePath, { cache: true }, function (_err: Error | null, res?: string) {

  it("parses a simple template w/ settings from Express", async () => {
    renderFile(
      filePath,
      {
        name: "<p>Ben</p>",
        cache: true,
        settings: {
          views: [path.join(__dirname, "templates"), path.join(__dirname, "othertemplates")],
          "view cache": true,
          "view options": { autoEscape: false },
        },
      },
      function (_err: Error | null, res?: string) {
        expect(res).toEqual("Hi <p>Ben</p>");
      }
    );
  });
    function cb(err: Error, _res?: string) {
      expect(err.message).toMatch(
    renderFile(errFilePath, { name: "Ada Lovelace", async: true }, cb);
Fixed code:  * - `renderFile(filename, data, [cb])`
  Express will call renderFile with (filename, data, cb)
  We also want to make (filename, data, options, cb) available
  data = data || {};
    // Otherwise, get the default config
    renderConfig = getConfig({}) as EtaConfigWithFilename;
 * - `renderFile(filename, data, [cb])`
 * - `renderFile(filename, data, [cb])`
  Express will call renderFile with (filename, data, cb)
  We also want to make (filename, data, options, cb) available
  data = data || {};
    // Otherwise, get the default config
    renderConfig = getConfig({}) as EtaConfigWithFilename;
 * - `renderFile(filename, data, [cb])`
    renderFile(fakeFilePath, {}, { cache: true }, function (_err: Error | null, res?: string) {
    function cb(err: Error | null, _res?: string) {
      expect(err?.message).toMatch(
    renderFile(errFilePath, { name: "Ada Lovelace" }, { async: true }, cb);