feat: add escapeHtml function

jsjoeio · jsjoeio · commit c505fc45a846 · 2021-07-01T10:43:36.000-07:00
This can be used to escape any special characters in a string with HTML before
sending from the server back to the client. This is important to prevent a
cross-site scripting attack.
diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts
@@ -4,7 +4,7 @@ import { RateLimiter as Limiter } from "limiter"
 import * as path from "path"
 import { rootPath } from "../constants"
 import { authenticated, getCookieDomain, redirect, replaceTemplates } from "../http"
-import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString } from "../util"
+import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString, escapeHtml } from "../util"
 
 export enum Cookie {
   Key = "key",
@@ -36,6 +36,7 @@ const getRoot = async (req: Request, error?: Error): Promise<string> => {
   } else if (req.args.usingEnvHashedPassword) {
     passwordMsg = "Password was set from $HASHED_PASSWORD."
   }
+
   return replaceTemplates(
     req,
     content
@@ -111,6 +112,8 @@ router.post("/", async (req, res) => {
 
     throw new Error("Incorrect password")
   } catch (error) {
-    res.send(await getRoot(req, error))
+    const html = await getRoot(req, error)
+    const escapedHtml = escapeHtml(html)
+    res.send(escapedHtml)
   }
 })
diff --git a/src/node/util.ts b/src/node/util.ts
@@ -508,3 +508,17 @@ export const isFile = async (path: string): Promise<boolean> => {
     return false
   }
 }
+
+/**
+ * Escapes any HTML string special characters, like &, <, >, ", and '.
+ *
+ * Source: https://stackoverflow.com/a/6234804/3015595
+ **/
+export function escapeHtml(unsafe: string): string {
+  return unsafe
+    .replace(/&/g, "&amp;")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;")
+    .replace(/'/g, "&#039;")
+}
diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts
@@ -445,3 +445,11 @@ describe("onLine", () => {
     expect(await received).toEqual(expected)
   })
 })
+
+describe("escapeHtml", () => {
+  it("should escape HTML", () => {
+    expect(util.escapeHtml(`<div class="error">"Hello & world"</div>`)).toBe(
+      "&lt;div class=&quot;error&quot;&gt;&quot;Hello &amp; world&quot;&lt;/div&gt;",
+    )
+  })
+})
diff --git a/test/unit/routes/login.test.ts b/test/unit/routes/login.test.ts
@@ -1,3 +1,6 @@
+import * as httpserver from "../../utils/httpserver"
+import * as integration from "../../utils/integration"
+
 import { RateLimiter } from "../../../src/node/routes/login"
 
 describe("login", () => {
@@ -34,4 +37,41 @@ describe("login", () => {
       expect(limiter.removeToken()).toBe(false)
     })
   })
+  describe("/login", () => {
+    let _codeServer: httpserver.HttpServer | undefined
+    function codeServer(): httpserver.HttpServer {
+      if (!_codeServer) {
+        throw new Error("tried to use code-server before setting it up")
+      }
+      return _codeServer
+    }
+
+    // Store whatever might be in here so we can restore it afterward.
+    // TODO: We should probably pass this as an argument somehow instead of
+    // manipulating the environment.
+    const previousEnvPassword = process.env.PASSWORD
+
+    beforeEach(async () => {
+      process.env.PASSWORD = "test"
+      _codeServer = await integration.setup(["--auth=password"], "")
+    })
+
+    afterEach(() => {
+      process.env.PASSWORD = previousEnvPassword
+    })
+
+    it("should return escaped HTML with 'Missing password' message", async () => {
+      const resp = await codeServer().fetch("/login", { method: "POST" })
+
+      expect(resp.status).toBe(200)
+
+      const htmlContent = await resp.text()
+
+      expect(htmlContent).not.toContain(">")
+      expect(htmlContent).not.toContain("<")
+      expect(htmlContent).not.toContain('"')
+      expect(htmlContent).not.toContain("'")
+      expect(htmlContent).toContain("&lt;div class=&quot;error&quot;&gt;Missing password&lt;/div&gt;")
+    })
+  })
 })
