fix: sanitize password and cookie key

Author: jsjoeio

ci/build/build-standalone-release.sh

@@ -3,7 +3,7 @@ set -euo pipefail
 
 # This is due to an upstream issue with RHEL7/CentOS 7 comptability with node-argon2
 # See: https://github.com/cdr/code-server/pull/3422#pullrequestreview-677765057
-npm_config_build_from_source=true
+export npm_config_build_from_source=true
 
 main() {
   cd "$(dirname "${0}")/../.."

ci/build/npm-postinstall.sh

@@ -20,7 +20,7 @@ detect_arch() {
 ARCH="${NPM_CONFIG_ARCH:-$(detect_arch)}"
 # This is due to an upstream issue with RHEL7/CentOS 7 comptability with node-argon2
 # See: https://github.com/cdr/code-server/pull/3422#pullrequestreview-677765057
-npm_config_build_from_source=true
+export npm_config_build_from_source=true
 
 main() {
   # Grabs the major version of node from $npm_config_user_agent which looks like

src/node/http.ts

@@ -7,7 +7,7 @@ import { normalize, Options } from "../common/util"
 import { AuthType, DefaultedArgs } from "./cli"
 import { commit, rootPath } from "./constants"
 import { Heart } from "./heart"
-import { getPasswordMethod, IsCookieValidArgs, isCookieValid } from "./util"
+import { getPasswordMethod, IsCookieValidArgs, isCookieValid, sanitizeString } from "./util"
 
 declare global {
   // eslint-disable-next-line @typescript-eslint/no-namespace
@@ -72,7 +72,7 @@ export const authenticated = async (req: express.Request): Promise<boolean> => {
       const passwordMethod = getPasswordMethod(hashedPasswordFromArgs)
       const isCookieValidArgs: IsCookieValidArgs = {
         passwordMethod,
-        cookieKey: req.cookies.key as string,
+        cookieKey: sanitizeString(req.cookies.key),
         passwordFromArgs: req.args.password || "",
         hashedPasswordFromArgs: req.args["hashed-password"],
       }

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 } from "../util"
+import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString } from "../util"
 
 export enum Cookie {
   Key = "key",
@@ -61,7 +61,7 @@ router.get("/", async (req, res) => {
 })
 
 router.post("/", async (req, res) => {
-  const password = req.body.password
+  const password = sanitizeString(req.body.password)
   const hashedPasswordFromArgs = req.args["hashed-password"]
 
   try {

src/node/util.ts

@@ -274,6 +274,17 @@ export async function isCookieValid(isCookieValidArgs: IsCookieValidArgs): Promi
   return isValid
 }
 
+/** Ensures that the input is sanitized by checking
+ * - it's a string
+ * - greater than 0 characters
+ * - trims whitespace
+ */
+export function sanitizeString(str: string): string {
+  // Very basic sanitization of string
+  // Credit: https://stackoverflow.com/a/46719000/3015595
+  return typeof str === "string" && str.trim().length > 0 ? str.trim() : ""
+}
+
 const mimeTypes: { [key: string]: string } = {
   ".aac": "audio/x-aac",
   ".avi": "video/x-msvideo",

test/unit/node/util.test.ts

@@ -7,6 +7,7 @@ import {
   hashLegacy,
   isHashLegacyMatch,
   isCookieValid,
+  sanitizeString,
 } from "../../../src/node/util"
 
 describe("getEnvPaths", () => {
@@ -382,3 +383,15 @@ describe.only("isCookieValid", () => {
     expect(isValid).toBe(false)
   })
 })
+
+describe.only("sanitizeString", () => {
+  it("should return an empty string if passed a type other than a string", () => {
+    expect(sanitizeString({} as string)).toBe("")
+  })
+  it("should trim whitespace", () => {
+    expect(sanitizeString(" hello   ")).toBe("hello")
+  })
+  it("should always return an empty string", () => {
+    expect(sanitizeString("   ")).toBe("")
+  })
+})