Disallow anchors/aliases to objects when loading config YAML

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");

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}"`);

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;
+}

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 });
+});

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<T extends {}>(obj: T, seen?: WeakSet<any>): 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);
+  }
+}