diff --git a/api/src/config.js b/api/src/config.js index 5dd65ca2..22610801 100644 --- a/api/src/config.js +++ b/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(); } diff --git a/api/src/routes/config.js b/api/src/routes/config.js index 7eecdf70..d3555727 100644 --- a/api/src/routes/config.js +++ b/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))) diff --git a/api/test/api-error-integration.test.js b/api/test/api-error-integration.test.js index 8af29e83..938cf9c6 100644 --- a/api/test/api-error-integration.test.js +++ b/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 })); diff --git a/api/test/config-durability.test.js b/api/test/config-durability.test.js index 79a43331..bc7fdc0d 100644 --- a/api/test/config-durability.test.js +++ b/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); +});