diff --git a/backend/src/api/guilds.ts b/backend/src/api/guilds.ts index 8969b29c..6f6a7a85 100644 --- a/backend/src/api/guilds.ts +++ b/backend/src/api/guilds.ts @@ -1,6 +1,6 @@ import { ApiPermissions } from "@shared/apiPermissions"; import express, { Request, Response } from "express"; -import yaml, { YAMLException } from "js-yaml"; +import { YAMLException } from "js-yaml"; import { validateGuildConfig } from "../configValidator"; import { AllowedGuilds } from "../data/AllowedGuilds"; import { ApiPermissionAssignments } from "../data/ApiPermissionAssignments"; @@ -8,6 +8,8 @@ import { Configs } from "../data/Configs"; import { apiTokenAuthHandlers } from "./auth"; import { hasGuildPermission, requireGuildPermission } from "./permissions"; import { clientError, ok, serverError, unauthorized } from "./responses"; +import { loadYamlSafely } from "../utils/loadYamlSafely"; +import { ObjectAliasError } from "../utils/validateNoObjectAliases"; const apiPermissionAssignments = new ApiPermissionAssignments(); @@ -61,12 +63,16 @@ export function initGuildsAPI(app: express.Express) { // Validate config let parsedConfig; try { - parsedConfig = yaml.safeLoad(config); + parsedConfig = loadYamlSafely(config); } catch (e) { if (e instanceof YAMLException) { return res.status(400).json({ errors: [e.message] }); } + if (e instanceof ObjectAliasError) { + return res.status(400).json({ errors: [e.message] }); + } + // tslint:disable-next-line:no-console console.error("Error when loading YAML: " + e.message); return serverError(res, "Server error"); diff --git a/backend/src/index.ts b/backend/src/index.ts index bfbab3f0..33043b24 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -19,6 +19,7 @@ import { SimpleError } from "./SimpleError"; import { ZeppelinGlobalConfig, ZeppelinGuildConfig } from "./types"; import { startUptimeCounter } from "./uptime"; import { errorMessage, isDiscordAPIError, isDiscordHTTPError, successMessage } from "./utils"; +import { loadYamlSafely } from "./utils/loadYamlSafely"; if (!process.env.KEY) { // tslint:disable-next-line:no-console @@ -209,7 +210,12 @@ connect().then(async () => { const key = id === "global" ? "global" : `guild-${id}`; const row = await guildConfigs.getActiveByKey(key); if (row) { - return yaml.safeLoad(row.config); + try { + return loadYamlSafely(row.config); + } catch (err) { + logger.error(`Error while loading config "${key}": ${err.message}`); + return {}; + } } logger.warn(`No config with key "${key}"`); diff --git a/backend/src/utils/loadYamlSafely.ts b/backend/src/utils/loadYamlSafely.ts new file mode 100644 index 00000000..b18148b9 --- /dev/null +++ b/backend/src/utils/loadYamlSafely.ts @@ -0,0 +1,11 @@ +import yaml from "js-yaml"; +import { validateNoObjectAliases } from "./validateNoObjectAliases"; + +/** + * Loads a YAML file safely while removing object anchors/aliases (including arrays) + */ +export function loadYamlSafely(yamlStr: string): any { + const loaded = yaml.safeLoad(yamlStr); + validateNoObjectAliases(loaded); + return loaded; +} diff --git a/backend/src/utils/validateNoObjectAliases.test.ts b/backend/src/utils/validateNoObjectAliases.test.ts new file mode 100644 index 00000000..fe2ff6bf --- /dev/null +++ b/backend/src/utils/validateNoObjectAliases.test.ts @@ -0,0 +1,43 @@ +import test from "ava"; +import { ObjectAliasError, validateNoObjectAliases } from "./validateNoObjectAliases"; + +test("validateNoObjectAliases() disallows object aliases at top level", t => { + const obj: any = { + objectRef: { + foo: "bar", + }, + }; + obj.otherProp = obj.objectRef; + + t.throws(() => validateNoObjectAliases(obj), { instanceOf: ObjectAliasError }); +}); + +test("validateNoObjectAliases() disallows aliases to nested objects", t => { + const obj: any = { + nested: { + objectRef: { + foo: "bar", + }, + }, + }; + obj.otherProp = obj.nested.objectRef; + + t.throws(() => validateNoObjectAliases(obj), { instanceOf: ObjectAliasError }); +}); + +test("validateNoObjectAliases() disallows nested object aliases", t => { + const obj: any = { + nested: { + objectRef: { + foo: "bar", + }, + }, + }; + obj.otherProp = { + alsoNested: { + ref: obj.nested.objectRef, + }, + }; + + t.throws(() => validateNoObjectAliases(obj), { instanceOf: ObjectAliasError }); +}); diff --git a/backend/src/utils/validateNoObjectAliases.ts b/backend/src/utils/validateNoObjectAliases.ts new file mode 100644 index 00000000..3666274d --- /dev/null +++ b/backend/src/utils/validateNoObjectAliases.ts @@ -0,0 +1,27 @@ +import { Not } from "../utils"; + +const scalarTypes = ["string", "number", "boolean", "bigint"]; + +export class ObjectAliasError extends Error {} + +/** + * Removes object aliases/anchors from a loaded YAML object + */ +export function validateNoObjectAliases(obj: T, seen?: WeakSet): void { + if (!seen) { + seen = new WeakSet(); + } + + for (const [key, value] of Object.entries(obj)) { + if (value == null || scalarTypes.includes(typeof value)) { + continue; + } + + if (seen.has(value)) { + throw new ObjectAliasError("Object aliases are not allowed"); + } + + validateNoObjectAliases(value as {}, seen); + seen.add(value); + } +}