From c505fc45a84623dc76dfb2884bf8229f17d50af1 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Tue, 29 Jun 2021 15:28:44 -0700 Subject: [PATCH] feat: add escapeHtml function 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. --- src/node/routes/login.ts | 7 ++++-- src/node/util.ts | 14 ++++++++++++ test/unit/node/util.test.ts | 8 +++++++ test/unit/routes/login.test.ts | 40 ++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts index dfd07ce9..2b160f25 100644 --- 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 => { } 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 index 5cb5e3cd..09e439de 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -508,3 +508,17 @@ export const isFile = async (path: string): Promise => { 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, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'") +} diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts index 8fae54b7..d089908b 100644 --- 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(`
"Hello & world"
`)).toBe( + "<div class="error">"Hello & world"</div>", + ) + }) +}) diff --git a/test/unit/routes/login.test.ts b/test/unit/routes/login.test.ts index 2a2e2046..9d68799b 100644 --- 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("<div class="error">Missing password</div>") + }) + }) })