Skip to content

Commit

Permalink
don't use sanitization for paths but validate
Browse files Browse the repository at this point in the history
related to #472
  • Loading branch information
MrBrax committed Nov 5, 2023
1 parent 87a4193 commit 7c73f06
Show file tree
Hide file tree
Showing 5 changed files with 322 additions and 22 deletions.
30 changes: 20 additions & 10 deletions common/Format.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
// https://stackoverflow.com/questions/7975005/format-a-javascript-string-using-placeholders-and-an-object-of-substitutions
export function formatString(string: string, replacements: Record<string, string>, hideEmpty = false): string {
// return string.replace(/{(\d+)}/g, (match, number) => {
// return typeof args[number] !== "undefined" ? args[number] : match;
// });
/**
* Replaces placeholders in a string with corresponding values from a replacements object.
* @param string - The string containing placeholders to replace.
* @param replacements - An object containing key-value pairs where the key is the placeholder and the value is the replacement.
* @param hideEmpty - If true, empty placeholders will be replaced with an empty string. Otherwise, they will be left as is.
* @returns The string with placeholders replaced with corresponding values from the replacements object.
*/
export function formatString(
string: string,
replacements: Record<string, string>,
hideEmpty = false
): string {
return string.replace(
/{(\w+)}/g,
/{(\w+)}/g,
(placeholderWithDelimiters, placeholderWithoutDelimiters) =>
hideEmpty ?
replacements[placeholderWithoutDelimiters] !== undefined ? replacements[placeholderWithoutDelimiters] : ""
:
replacements[placeholderWithoutDelimiters] || placeholderWithDelimiters
hideEmpty
? replacements[placeholderWithoutDelimiters] !== undefined
? replacements[placeholderWithoutDelimiters]
: ""
: replacements[placeholderWithoutDelimiters] ||
placeholderWithDelimiters
);
}
}
56 changes: 44 additions & 12 deletions server/src/Controllers/Channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import { YouTubeChannel } from "@/Core/Providers/YouTube/YouTubeChannel";
import { YouTubeVOD } from "@/Core/Providers/YouTube/YouTubeVOD";
import { Webhook } from "@/Core/Webhook";
import { debugLog } from "@/Helpers/Console";
import {
validateAbsolutePath,
validateFilename,
validateRelativePath,
} from "@/Helpers/Filesystem";
import { generateStreamerList } from "@/Helpers/StreamerList";
import { isError, isTwitchChannel, isYouTubeChannel } from "@/Helpers/Types";
import type {
Expand All @@ -40,7 +45,6 @@ import type express from "express";
import { randomUUID } from "node:crypto";
import fs from "node:fs";
import path from "node:path";
import sanitize from "sanitize-filename";
import { TwitchHelper } from "../Providers/Twitch";
import type { TwitchVODChapterJSON } from "../Storage/JSON";

Expand Down Expand Up @@ -692,9 +696,7 @@ export async function DownloadVideo(
game_id: extraData.game_id || "",
};

return sanitize(
formatString(Config.getInstance().cfg(what), variables)
);
return formatString(Config.getInstance().cfg(what), variables);
};

if (isTwitchChannel(channel)) {
Expand Down Expand Up @@ -752,19 +754,49 @@ export async function DownloadVideo(
"filename_vod"
);

if (!validateFilename(basename)) {
res.api(400, {
status: "ERROR",
message: req.t("message.invalid-filesystem-entry", [basename]),
} as ApiErrorResponse);
return;
}

const basefolderPathTemplate = template(
video,
{
game_name: videoGqlData?.game?.displayName || "",
game_id: videoGqlData?.game?.id || "",
},
"filename_vod_folder"
);

if (!validateRelativePath(basefolderPathTemplate)) {
res.api(400, {
status: "ERROR",
message: req.t("message.invalid-filesystem-entry", [
basefolderPathTemplate,
]),
} as ApiErrorResponse);
return;
}

// make folder name
const basefolder = path.join(
channel.getFolder(),
template(
video,
{
game_name: videoGqlData?.game?.displayName || "",
game_id: videoGqlData?.game?.id || "",
},
"filename_vod_folder"
)
basefolderPathTemplate
);

if (!validateAbsolutePath(basefolder)) {
res.api(400, {
status: "ERROR",
message: req.t("message.invalid-filesystem-entry", [
basefolder,
]),
} as ApiErrorResponse);
return;
}

// make filepath
const filepath = path.join(
basefolder,
Expand Down
56 changes: 56 additions & 0 deletions server/src/Helpers/Filesystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,59 @@ export function directorySize(dir: string): number {
}
return size;
}

/*
export function sanitizeAbsolutePath(dir: string): string {
}
export function sanitizeRelativePath(dir: string): string {
dir = path.normalize(dir);
// linux
if (dir.startsWith("/")) {
dir = dir.substring(1);
}
// windows
if (dir.match(/^[a-zA-Z]:\\/)) {
dir = dir.substring(3);
}
}
export function sanitizeFilename(filename: string): string {
return filename.replace(/[\\/:*?"<>|]/g, "_");
}
*/

export function validateAbsolutePath(dir: string): boolean {
return path.isAbsolute(dir) && !dir.match(/\0/);
}

export function validateRelativePath(dir: string): boolean {
return (
!path.isAbsolute(dir) &&
// windows drive
!dir.match(/^[a-zA-Z]:\\/) &&
// linux root
!dir.startsWith("/") &&
// parent directory, but double dots can be part of the filename
!dir.match(/[^\\]\\\.\.($|\\)/) &&
!dir.startsWith("..\\") &&
!dir.startsWith("../") &&
// current directory
!dir.match(/[^\\]\\\.($|\\)/) &&
// null character
!dir.match(/\0/)
);
}

export function validateFilename(filename: string): boolean {
return !/[\\/:*?"<>|\0]/.test(filename);
}
199 changes: 199 additions & 0 deletions server/src/Helpers/Format.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@ import {
getNiceDuration,
} from "./Format";

import { formatString } from "@common/Format";
import path from "path";

import {
validateAbsolutePath,
validateFilename,
validateRelativePath,
} from "./Filesystem";

describe("Format Helpers", () => {
describe("formatDuration", () => {
it("should format duration in HH:MM:SS", () => {
Expand Down Expand Up @@ -53,3 +62,193 @@ describe("Format Helpers", () => {
});
});
});
describe("Format Helpers", () => {
describe("formatString", () => {
it("should replace placeholders with values from replacements object", () => {
const string = "Hello {name}, your age is {age}.";
const replacements = { name: "John", age: "30" };
expect(formatString(string, replacements)).toEqual(
"Hello John, your age is 30."
);
});

it("should replace multiple placeholders with values from replacements object", () => {
const string =
"Hello {name}, your age is {age}. You live in {city}.";
const replacements = { name: "John", age: "30", city: "New York" };
expect(formatString(string, replacements)).toEqual(
"Hello John, your age is 30. You live in New York."
);
});

it("should replace placeholders with empty string if hideEmpty is true and value is undefined", () => {
const string = "Hello {name}, your age is {age}.";
const replacements = { name: "John" };
expect(formatString(string, replacements, true)).toEqual(
"Hello John, your age is ."
);
});

it("should not replace placeholders with empty string if hideEmpty is false and value is undefined", () => {
const string = "Hello {name}, your age is {age}.";
const replacements = { name: "John" };
expect(formatString(string, replacements)).toEqual(
"Hello John, your age is {age}."
);
});

it("should not replace placeholders if replacements object does not contain the key", () => {
const string = "Hello {name}, your age is {age}.";
const replacements = { name: "John", city: "New York" };
expect(formatString(string, replacements)).toEqual(
"Hello John, your age is {age}."
);
});
});
});

/*
describe("Sanitize", () => {
it("should sanitize absolute directory path", () => {
expect(sanitizeAbsolutePath(path.join("C:\\", "test", "test"))).toEqual(
"C:\\test\\test"
);
expect(
sanitizeAbsolutePath(path.join("C:\\", "test", "test\\"))
).toEqual("C:\\test\\test");
expect(
sanitizeAbsolutePath(path.join("C:\\", "test", "test\\\\"))
).toEqual("C:\\test\\test");
expect(sanitizeAbsolutePath(path.join("C:\\", "test/test"))).toEqual(
"C:\\test\\test"
);
});
it("should sanitize relative directory path", () => {
expect(sanitizeRelativePath(path.join("test", "test"))).toEqual(
path.join("test", "test")
);
expect(sanitizeRelativePath(path.join("test", "test\\"))).toEqual(
path.join("test", "test")
);
expect(sanitizeRelativePath(path.join("test", "test\\\\"))).toEqual(
path.join("test", "test")
);
expect(sanitizeRelativePath(path.join("test/test"))).toEqual(
path.join("test", "test")
);
});
it("should sanitize filename", () => {
expect(sanitizeFilename("test")).toEqual("test");
expect(sanitizeFilename("test.txt")).toEqual("test.txt");
expect(sanitizeFilename("../test.txt")).toEqual("test.txt");
expect(sanitizeFilename("test/test.txt")).toEqual("testtest.txt");
expect(sanitizeFilename("C:\\test\\test.txt")).toEqual("Ctesttest.txt");
expect(sanitizeFilename("/test/test.txt")).toEqual("testtest.txt");
});
});
*/

describe("Validate", () => {
describe("validateAbsolutePath", () => {
it("should validate absolute directory path", () => {
expect(
validateAbsolutePath(path.join("C:\\", "test", "test"))
).toEqual(true);
expect(
validateAbsolutePath(path.join("C:\\", "test", "test\\"))
).toEqual(true);
expect(
validateAbsolutePath(path.join("C:\\", "test", "test\\\\"))
).toEqual(true);
expect(
validateAbsolutePath(path.join("C:\\", "test/test"))
).toEqual(true);
expect(validateAbsolutePath(path.join("C:\\\0\\test"))).toEqual(
false
);
});

it("should not validate relative directory path", () => {
expect(validateAbsolutePath(path.join("test", "test"))).toEqual(
false
);
expect(validateAbsolutePath(path.join("test", "test\\"))).toEqual(
false
);
expect(validateAbsolutePath(path.join("test", "test\\\\"))).toEqual(
false
);
expect(validateAbsolutePath(path.join("test/test"))).toEqual(false);
});
});

describe("validateRelativePath", () => {
it("should not validate absolute directory path", () => {
expect(
validateRelativePath(path.join("C:\\", "test", "test"))
).toEqual(false);
expect(
validateRelativePath(path.join("C:\\", "test", "test\\"))
).toEqual(false);
expect(
validateRelativePath(path.join("C:\\", "test", "test\\\\"))
).toEqual(false);
expect(
validateRelativePath(path.join("C:\\", "test/test"))
).toEqual(false);
expect(validateRelativePath(path.join("/test", "test"))).toEqual(
false
);
expect(validateRelativePath(path.join("/test\0", "test"))).toEqual(
false
);
});

it("should validate relative directory path", () => {
expect(validateRelativePath(path.join("test", "test"))).toEqual(
true
);
expect(validateRelativePath(path.join("test", "test\\"))).toEqual(
true
);
expect(validateRelativePath(path.join("test", "test\\\\"))).toEqual(
true
);
expect(validateRelativePath(path.join("test/test"))).toEqual(true);
});

it("should fail relative directory path with ..", () => {
expect(validateRelativePath(path.join("..", "test"))).toEqual(
false
);
expect(validateRelativePath(path.join("..", "test\\"))).toEqual(
false
);
expect(validateRelativePath(path.join("..", "test\\\\"))).toEqual(
false
);
expect(validateRelativePath(path.join("..", "test/test"))).toEqual(
false
);
expect(validateRelativePath("test../test")).toEqual(true);
});
});

describe("validateFilename", () => {
it("should validate filename", () => {
expect(validateFilename("test")).toEqual(true);
expect(validateFilename("test.txt")).toEqual(true);
expect(validateFilename("test..txt")).toEqual(true);
expect(validateFilename("../test.txt")).toEqual(false);
expect(validateFilename("test/test.txt")).toEqual(false);
expect(validateFilename("C:\\test\\test.txt")).toEqual(false);
expect(validateFilename("/test/test.txt")).toEqual(false);
expect(
validateFilename("file_[1]_(super).{brackets}-dash.test.txt")
).toEqual(true);
expect(validateFilename("\0test.txt")).toEqual(false);
});
});
});
Loading

0 comments on commit 7c73f06

Please sign in to comment.