diff --git a/ci/build/build-standalone-release.sh b/ci/build/build-standalone-release.sh index a8072a5a..3afde2ea 100755 --- a/ci/build/build-standalone-release.sh +++ b/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}")/../.." diff --git a/ci/build/npm-postinstall.sh b/ci/build/npm-postinstall.sh index ae1be016..5e413d7a 100755 --- a/ci/build/npm-postinstall.sh +++ b/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 diff --git a/src/node/http.ts b/src/node/http.ts index 8b4c9d8c..87a0be10 100644 --- a/src/node/http.ts +++ b/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 => { 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"], } diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts index 6b8ab4d2..dfd07ce9 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 } 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 { diff --git a/src/node/util.ts b/src/node/util.ts index 5a3d0b70..6fbabd0f 100644 --- a/src/node/util.ts +++ b/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", diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts index 0397fbff..14a67445 100644 --- a/test/unit/node/util.test.ts +++ b/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("") + }) +})