Security: refuse deeply nested config saves and clone before writing

api/src/config.js

@@ -282,6 +282,9 @@ function saveConfig(data, { keepRev = false } = {}) {
     data._schemaVersion = SCHEMA_VERSION;
     if (!keepRev) data._rev = (Number(data._rev) || 0) + 1;
   }
+  /* Clone before writing. A value the clone cannot handle must fail here, not
+     after the file is replaced: every later read clones it and fails too. */
+  const copy = structuredClone(data);
   const dir = path.dirname(CONFIG_PATH);
   fs.mkdirSync(dir, { recursive: true });
   const tmp = CONFIG_PATH + '.tmp';
@@ -320,8 +323,8 @@ function saveConfig(data, { keepRev = false } = {}) {
        durable. */
   }
 
-  /* Only after the write succeeded. A copy: the caller keeps its object. */
-  _cfgCache = structuredClone(data);
+  /* Only after the write succeeded. */
+  _cfgCache = copy;
   _cfgCacheAt = Date.now();
 }
 

api/src/routes/config.js

@@ -15,6 +15,16 @@ const { stripDisabledCredentials } = require('../auth');
 const { pruneWallpapers } = require('./wallpaper');
 const { normalizeHostList } = require('../../../ui/js/host-names.js');
 
+/* Real configs nest under 10 levels. Far deeper input is written to disk and
+   then breaks every read of the config. */
+const MAX_DEPTH = 32;
+function tooDeep(v, depth = 0) {
+  if (!v || typeof v !== 'object') return false;
+  if (depth >= MAX_DEPTH) return true;
+  for (const k in v) if (tooDeep(v[k], depth + 1)) return true;
+  return false;
+}
+
 function scrubSecrets(cfg) {
   const safe = structuredClone(cfg);
   scrubAllSecrets(safe);
@@ -59,6 +69,8 @@ on('POST', '/api/config', async (req, res) => {
   if (!checkOrigin(req, res)) return;
   try {
     const data = JSON.parse(await readBody(req));
+    if (tooDeep(data))
+      return json(res, 400, { error: `config nests deeper than ${MAX_DEPTH} levels`, kind: KIND.INVALID });
     if (!Array.isArray(data.items)) return json(res, 400, { error: 'items must be an array', kind: KIND.INVALID });
     /* An array or a primitive here drops the stored password on write. */
     if (data.settings != null && (typeof data.settings !== 'object' || Array.isArray(data.settings)))

api/test/api-error-integration.test.js

@@ -227,6 +227,25 @@ test('a rejected config save is tagged invalid, with its message intact', async
   assert.equal(r.body.error, 'items must be an array');
 });
 
+test('a config save nested too deep is refused and leaves the file alone', async () => {
+  const onDisk = fs.readFileSync(process.env.CONFIG_PATH, 'utf8');
+  const deep = '['.repeat(5000) + '1' + ']'.repeat(5000);
+  const r = await post('/api/config', `{"items":[{"id":"a","type":"app","note":${deep}}],"settings":{}}`, {
+    raw: true,
+  });
+  assert.equal(r.status, 400);
+  assert.equal(r.body.kind, KIND.INVALID);
+  assert.equal(fs.readFileSync(process.env.CONFIG_PATH, 'utf8'), onDisk);
+});
+
+test('a config save at a realistic depth is accepted', async () => {
+  const r = await post('/api/config', {
+    items: [{ id: 'a', type: 'widget', widgetConfig: { shelves: [{ rows: [{ cells: [{ v: 1 }] }] }] } }],
+    settings: {},
+  });
+  assert.equal(r.status, 200);
+});
+
 test('a malformed body never reaches the log', async t => {
   const logged = [];
   t.mock.method(log, 'error', (msg, data) => logged.push({ msg, data }));

api/test/config-durability.test.js

@@ -167,3 +167,12 @@ test('a successful save bumps the revision', () => {
   saveConfig(loadConfig());
   assert.equal(loadConfig()._rev, before + 1);
 });
+
+test('a value too deep to clone is refused before the file changes', () => {
+  const before = fs.readFileSync(process.env.CONFIG_PATH, 'utf8');
+  let deep = 1;
+  for (let i = 0; i < 5000; i++) deep = [deep];
+  assert.throws(() => saveConfig({ items: [{ id: 'a', type: 'app', note: deep }], settings: {} }));
+  assert.equal(fs.readFileSync(process.env.CONFIG_PATH, 'utf8'), before);
+  assert.equal(fs.existsSync(TMP), false);
+});