From c489497e451887bfe400434c5a010940051e9890 Mon Sep 17 00:00:00 2001 From: Erik Michelson Date: Tue, 9 Jun 2026 22:09:03 +0200 Subject: [PATCH] refactor(frontmatter): constrain frontmatter object after parsing This change removes the meta-marked dependency which solely was used for extracting the frontmatter, which is possible as well with one function. Furthermore, this introduces constraints to objects resulting from frontmatter parsing and enforces them in order to prevent attacks like a yaml bomb (massive alias expansion). This change should resolve a possible DoS attack. Signed-off-by: Erik Michelson --- lib/models/note.js | 38 +++++++- package.json | 4 +- public/docs/release-notes.md | 4 + public/js/extra.js | 6 +- public/views/includes/scripts.ejs | 1 + public/views/pretty.ejs | 2 +- test/note.js | 138 ++++++++++++++++++++++++++++++ webpack.common.js | 12 ++- webpack.prod.js | 2 +- yarn.lock | 69 ++++----------- 10 files changed, 212 insertions(+), 64 deletions(-) create mode 100644 test/note.js diff --git a/lib/models/note.js b/lib/models/note.js index 19c0694c..becc947b 100644 --- a/lib/models/note.js +++ b/lib/models/note.js @@ -5,12 +5,13 @@ const path = require('path') const LZString = require('lz-string') const base64url = require('base64url') const md = require('markdown-it')() -const metaMarked = require('@hedgedoc/meta-marked') +const yaml = require('js-yaml') const cheerio = require('cheerio') const nanoid = require('nanoid') const Sequelize = require('sequelize') const async = require('async') const moment = require('moment') +const { constrainObject } = require('constrain-object') const DiffMatchPatch = require('diff-match-patch') const dmp = new DiffMatchPatch() @@ -397,12 +398,41 @@ module.exports = function (sequelize, DataTypes) { } return tags } + function splitFrontmatter (str) { + if (str.slice(0, 3) !== '---') { + return null + } + const matcher = /\n(\.{3}|-{3})/g + const metaEnd = matcher.exec(str) + if (!metaEnd) { + return null + } + return { + yaml: str.slice(0, metaEnd.index), + markdown: str.slice(matcher.lastIndex) + } + } + Note.extractMeta = function (content) { let obj = null try { - obj = metaMarked(content) - if (!obj.markdown) obj.markdown = '' - if (!obj.meta) obj.meta = {} + const parts = splitFrontmatter(content) + if (parts) { + const parsed = yaml.load(parts.yaml) + const constrained = constrainObject(parsed, { + onCycle: 'omit', + maxDepth: 4 + }) + obj = { + meta: constrained || {}, + markdown: parts.markdown || '' + } + } else { + obj = { + meta: {}, + markdown: content + } + } } catch (err) { obj = { markdown: content, diff --git a/package.json b/package.json index a6e077a5..9a6c499c 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,6 @@ "start": "node app.js" }, "dependencies": { - "@hedgedoc/meta-marked": "14.1.0", "@node-saml/passport-saml": "5.1.0", "@passport-next/passport-openid": "1.0.0", "Idle.Js": "git+https://github.com/shawnmclean/Idle.js#commit=2b57cc6e49d177b7ddce0cca00ef5cbe07453541", @@ -34,6 +33,7 @@ "compression": "1.8.1", "connect-flash": "0.1.1", "connect-session-sequelize": "8.0.6", + "constrain-object": "1.1.0", "cookie": "1.1.1", "cookie-parser": "1.4.7", "deep-freeze": "0.0.1", @@ -49,6 +49,7 @@ "helmet": "8.2.0", "i18n": "0.15.3", "is-svg": "4.4.0", + "js-yaml": "4.2.0", "jsdom": "24.1.3", "lodash": "4.18.1", "lutim": "1.0.3", @@ -175,7 +176,6 @@ "jquery-ui": "1.14.2", "js-cookie": "3.0.8", "js-sequence-diagrams": "git+https://github.com/hedgedoc/js-sequence-diagrams.git", - "js-yaml": "3.14.2", "jsonlint": "1.6.3", "keymaster": "1.6.2", "less": "4.6.6", diff --git a/public/docs/release-notes.md b/public/docs/release-notes.md index 5916b9f7..40cd4f15 100644 --- a/public/docs/release-notes.md +++ b/public/docs/release-notes.md @@ -16,6 +16,10 @@ Thanks to [Chandler Johnson](https://github.com/chndlrx) for reporting! - Allow configuration of login / signup rate-limits - Allow configuration of Cloudflare usage in regards rate-limits +### Bugfixes + +- Made frontmatter parsing resilient to massive YAML alias expansion ("YAML bomb") + ## 1.10.8 2026-04-15 ### Bugfixes diff --git a/public/js/extra.js b/public/js/extra.js index cba0bfeb..7103ff8a 100644 --- a/public/js/extra.js +++ b/public/js/extra.js @@ -1247,7 +1247,11 @@ function meta (state, start, end, silent) { if (line >= end) return false try { - md.meta = window.jsyaml.safeLoad(data.join('\n')) || {} + const parsed = window.jsyaml.load(data.join('\n')) || {} + md.meta = window.ConstrainObject.constrainObject(parsed, { + onCycle: 'omit', + maxDepth: 4 + }) delete md.metaError } catch (err) { md.metaError = err diff --git a/public/views/includes/scripts.ejs b/public/views/includes/scripts.ejs index 259330af..db2a3ef8 100644 --- a/public/views/includes/scripts.ejs +++ b/public/views/includes/scripts.ejs @@ -1,4 +1,5 @@ + <% for (var js in htmlWebpackPlugin.files.js) { %> <% } %> diff --git a/public/views/pretty.ejs b/public/views/pretty.ejs index 5ee3645f..bddb1db5 100644 --- a/public/views/pretty.ejs +++ b/public/views/pretty.ejs @@ -15,7 +15,7 @@ <% } %> <% for (var og in opengraph) { %> - <% if (opengraph.hasOwnProperty(og) && opengraph[og].trim() !== '') { %> + <% if (opengraph.hasOwnProperty(og) && typeof opengraph[og] === 'string' && opengraph[og].trim() !== '') { %> <% }} if (!opengraph.hasOwnProperty('image')) { %> diff --git a/test/note.js b/test/note.js new file mode 100644 index 00000000..114d362f --- /dev/null +++ b/test/note.js @@ -0,0 +1,138 @@ +'use strict' + +const assert = require('assert') + +const models = require('../lib/models') +const Note = models.Note + +describe('Note.extractMeta', function () { + it('parses standard frontmatter with title and description', function () { + const content = '---\ntitle: My Note\ndescription: A test note\n---\n# Hello World' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.title, 'My Note') + assert.strictEqual(result.meta.description, 'A test note') + assert.strictEqual(result.markdown, '\n# Hello World') + }) + + it('parses frontmatter with tags as array', function () { + const content = '---\ntitle: Tagged\ntags:\n - foo\n - bar\n - baz\n---\nBody' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.title, 'Tagged') + assert.deepStrictEqual(result.meta.tags, ['foo', 'bar', 'baz']) + }) + + it('parses frontmatter with tags as string', function () { + const content = '---\ntags: foo, bar, baz\n---\nBody' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.tags, 'foo, bar, baz') + }) + + it('parses frontmatter with opengraph object', function () { + const content = '---\nopengraph:\n title: OG Title\n description: OG Description\n image: https://example.com/img.png\n---\nBody' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.opengraph.title, 'OG Title') + assert.strictEqual(result.meta.opengraph.description, 'OG Description') + assert.strictEqual(result.meta.opengraph.image, 'https://example.com/img.png') + }) + + it('parses frontmatter with slideOptions', function () { + const content = '---\nslideOptions:\n theme: moon\n transition: slide\n---\n# Slide 1' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.slideOptions.theme, 'moon') + assert.strictEqual(result.meta.slideOptions.transition, 'slide') + }) + + it('parses frontmatter with boolean and numeric values', function () { + const content = '---\nbreaks: false\nGA: UA-12345\nlang: en\n---\nBody' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.breaks, false) + assert.strictEqual(result.meta.GA, 'UA-12345') + assert.strictEqual(result.meta.lang, 'en') + }) + + it('returns empty meta and full content when no frontmatter present', function () { + const content = '# Just a heading\n\nSome text here.' + const result = Note.extractMeta(content) + assert.deepStrictEqual(result.meta, {}) + assert.strictEqual(result.markdown, content) + }) + + it('returns empty meta for empty frontmatter', function () { + const content = '---\n---\nBody text' + const result = Note.extractMeta(content) + assert.deepStrictEqual(result.meta, {}) + assert.strictEqual(result.markdown, '\nBody text') + }) + + it('handles frontmatter closed with ...', function () { + const content = '---\ntitle: Dot Delimiter\n...\nBody' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.title, 'Dot Delimiter') + assert.strictEqual(result.markdown, '\nBody') + }) + + it('returns empty meta for malformed YAML', function () { + const content = '---\ntitle: [invalid yaml\n---\nBody' + const result = Note.extractMeta(content) + assert.deepStrictEqual(result.meta, {}) + assert.strictEqual(result.markdown, content) + }) + + it('handles empty content', function () { + const result = Note.extractMeta('') + assert.deepStrictEqual(result.meta, {}) + assert.strictEqual(result.markdown, '') + }) + + it('handles content that starts with --- but has no closing delimiter', function () { + const content = '---\ntitle: No closing\nNo delimiter here' + const result = Note.extractMeta(content) + assert.deepStrictEqual(result.meta, {}) + assert.strictEqual(result.markdown, content) + }) + + it('neutralizes YAML alias bomb (10 levels of 10-way branching)', function () { + const content = [ + '---', + 'a0: &a0 1', + 'a1: &a1 [*a0,*a0,*a0,*a0,*a0,*a0,*a0,*a0,*a0,*a0]', + 'a2: &a2 [*a1,*a1,*a1,*a1,*a1,*a1,*a1,*a1,*a1,*a1]', + 'a3: &a3 [*a2,*a2,*a2,*a2,*a2,*a2,*a2,*a2,*a2,*a2]', + 'a4: &a4 [*a3,*a3,*a3,*a3,*a3,*a3,*a3,*a3,*a3,*a3]', + 'a5: &a5 [*a4,*a4,*a4,*a4,*a4,*a4,*a4,*a4,*a4,*a4]', + 'a6: &a6 [*a5,*a5,*a5,*a5,*a5,*a5,*a5,*a5,*a5,*a5]', + 'a7: &a7 [*a6,*a6,*a6,*a6,*a6,*a6,*a6,*a6,*a6,*a6]', + 'a8: &a8 [*a7,*a7,*a7,*a7,*a7,*a7,*a7,*a7,*a7,*a7]', + 'a9: &a9 [*a8,*a8,*a8,*a8,*a8,*a8,*a8,*a8,*a8,*a8]', + 'a10: &a10 [*a9,*a9,*a9,*a9,*a9,*a9,*a9,*a9,*a9,*a9]', + 'opengraph: *a10', + '---', + 'x' + ].join('\n') + + const start = Date.now() + const result = Note.extractMeta(content) + const elapsed = Date.now() - start + + assert.ok(elapsed < 1000, `extractMeta took ${elapsed}ms, should complete in under 1000ms`) + assert.ok(result.meta !== null, 'meta should not be null') + assert.ok(typeof result.meta === 'object', 'meta should be an object') + assert.strictEqual(result.markdown, '\nx') + + const json = JSON.stringify(result.meta) + assert.ok(json.length < 100000, `stringified meta should be small, got ${json.length} bytes`) + }) + + it('truncates excessively long string values', function () { + const longString = 'a'.repeat(20000) + const content = `---\ntitle: ${longString}\n---\nBody` + const result = Note.extractMeta(content) + assert.ok(result.meta.title.length <= 10000, `title should be truncated, got ${result.meta.title.length}`) + }) + + it('preserves normal nested objects within depth limit', function () { + const content = '---\nlevel1:\n level2:\n level3: deep value\n---\nBody' + const result = Note.extractMeta(content) + assert.strictEqual(result.meta.level1.level2.level3, 'deep value') + }) +}) diff --git a/webpack.common.js b/webpack.common.js index dd4de4e4..a70cca98 100644 --- a/webpack.common.js +++ b/webpack.common.js @@ -137,6 +137,11 @@ module.exports = { context: path.join(__dirname, 'node_modules/reveal.js'), from: 'plugin', to: 'reveal.js/plugin' + }, + { + context: path.join(__dirname, 'node_modules/constrain-object/dist'), + from: 'index.iife.js', + to: 'constrain-object.min.js' } ] }), @@ -301,11 +306,16 @@ module.exports = { 'socket.io-client': 'io', 'jquery': '$', 'moment': 'moment', - 'select2': 'select2' + 'select2': 'select2', + 'constrain-object': 'ConstrainObject' }, module: { rules: [ + { + test: /\.mjs$/, + type: 'javascript/auto' + }, { test: require.resolve(path.join(__dirname, 'public/vendor/codemirror-spell-checker/spell-checker.min.js')), use: { diff --git a/webpack.prod.js b/webpack.prod.js index cacdf30b..a3f49d2d 100644 --- a/webpack.prod.js +++ b/webpack.prod.js @@ -18,7 +18,7 @@ module.exports = [ new EsbuildPlugin({ target: 'es2015', format: 'cjs', - exclude: ['MathJax/extensions/a11y/mathmaps', 'reveal.js/plugin/markdown/marked.js'] + exclude: ['MathJax/extensions/a11y/mathmaps', 'reveal.js/plugin/markdown/marked.js', 'constrain-object.min.js'] }) ], splitChunks: { diff --git a/yarn.lock b/yarn.lock index 5b0d7cfc..c8a52844 100644 --- a/yarn.lock +++ b/yarn.lock @@ -412,16 +412,6 @@ __metadata: languageName: node linkType: hard -"@hedgedoc/meta-marked@npm:14.1.0": - version: 14.1.0 - resolution: "@hedgedoc/meta-marked@npm:14.1.0" - dependencies: - js-yaml: "npm:~4.1.0" - marked: "npm:14.1.0" - checksum: 10/f14edf09cd7d33e20ae1a0e8895688613691acf55cc204b96134bbc5a9ab85df4cc73deea26baebf897b18f315bb0e144df0a34519813aae9aaae9293cae2db2 - languageName: node - linkType: hard - "@humanfs/core@npm:^0.19.1": version: 0.19.1 resolution: "@humanfs/core@npm:0.19.1" @@ -1501,7 +1491,6 @@ __metadata: "@eslint/eslintrc": "npm:3.3.5" "@eslint/js": "npm:9.39.4" "@hedgedoc/codemirror-5": "npm:5.65.12" - "@hedgedoc/meta-marked": "npm:14.1.0" "@node-saml/passport-saml": "npm:5.1.0" "@passport-next/passport-openid": "npm:1.0.0" Idle.Js: "git+https://github.com/shawnmclean/Idle.js#commit=2b57cc6e49d177b7ddce0cca00ef5cbe07453541" @@ -1528,6 +1517,7 @@ __metadata: compression: "npm:1.8.1" connect-flash: "npm:0.1.1" connect-session-sequelize: "npm:8.0.6" + constrain-object: "npm:1.1.0" cookie: "npm:1.1.1" cookie-parser: "npm:1.4.7" copy-webpack-plugin: "npm:6.4.1" @@ -1569,7 +1559,7 @@ __metadata: jquery-ui: "npm:1.14.2" js-cookie: "npm:3.0.8" js-sequence-diagrams: "git+https://github.com/hedgedoc/js-sequence-diagrams.git" - js-yaml: "npm:3.14.2" + js-yaml: "npm:4.2.0" jsdom: "npm:24.1.3" jsonlint: "npm:1.6.3" keymaster: "npm:1.6.2" @@ -1967,15 +1957,6 @@ __metadata: languageName: node linkType: hard -"argparse@npm:^1.0.7": - version: 1.0.10 - resolution: "argparse@npm:1.0.10" - dependencies: - sprintf-js: "npm:~1.0.2" - checksum: 10/c6a621343a553ff3779390bb5ee9c2263d6643ebcd7843227bdde6cc7adbed796eb5540ca98db19e3fd7b4714e1faa51551f8849b268bb62df27ddb15cbcd91e - languageName: node - linkType: hard - "argparse@npm:^2.0.1": version: 2.0.1 resolution: "argparse@npm:2.0.1" @@ -4295,6 +4276,13 @@ __metadata: languageName: node linkType: hard +"constrain-object@npm:1.1.0": + version: 1.1.0 + resolution: "constrain-object@npm:1.1.0" + checksum: 10/0500dad5fd2d01c2c7269259bec731cfb0a2ee7dcef33d0cb92b3ff32dd15d60e47f1fd23da6678aa09f3d4ec3a032b99e2950364c6fa088c64d463c01fd3eec + languageName: node + linkType: hard + "content-disposition@npm:~0.5.4": version: 0.5.4 resolution: "content-disposition@npm:0.5.4" @@ -6643,16 +6631,6 @@ __metadata: languageName: node linkType: hard -"esprima@npm:^4.0.0": - version: 4.0.1 - resolution: "esprima@npm:4.0.1" - bin: - esparse: ./bin/esparse.js - esvalidate: ./bin/esvalidate.js - checksum: 10/f1d3c622ad992421362294f7acf866aa9409fbad4eb2e8fa230bd33944ce371d32279667b242d8b8907ec2b6ad7353a717f3c0e60e748873a34a7905174bc0eb - languageName: node - linkType: hard - "esquery@npm:^1.5.0": version: 1.7.0 resolution: "esquery@npm:1.7.0" @@ -9292,19 +9270,18 @@ __metadata: languageName: node linkType: hard -"js-yaml@npm:3.14.2": - version: 3.14.2 - resolution: "js-yaml@npm:3.14.2" +"js-yaml@npm:4.2.0": + version: 4.2.0 + resolution: "js-yaml@npm:4.2.0" dependencies: - argparse: "npm:^1.0.7" - esprima: "npm:^4.0.0" + argparse: "npm:^2.0.1" bin: js-yaml: bin/js-yaml.js - checksum: 10/172e0b6007b0bf0fc8d2469c94424f7dd765c64a047d2b790831fecef2204a4054eabf4d911eb73ab8c9a3256ab8ba1ee8d655b789bf24bf059c772acc2075a1 + checksum: 10/51de2067a2b44b07ba5206132e56005f8b568ff279bb4d2f645068958c56fa4827d40a6841c983234671fa0a134bf094d0b0717873c2a3d319185297af145a6d languageName: node linkType: hard -"js-yaml@npm:^4.1.0, js-yaml@npm:^4.1.1, js-yaml@npm:~4.1.0": +"js-yaml@npm:^4.1.0, js-yaml@npm:^4.1.1": version: 4.1.1 resolution: "js-yaml@npm:4.1.1" dependencies: @@ -10247,15 +10224,6 @@ __metadata: languageName: node linkType: hard -"marked@npm:14.1.0": - version: 14.1.0 - resolution: "marked@npm:14.1.0" - bin: - marked: bin/marked.js - checksum: 10/1a930dd87a3994cc4fcc72c4668c548429d0e6363b8f7660193c106fa1cadcde5c813cfc3fe4be42b9f18b3652ba73469fb6c718215b0dbf8ddb9de2d1f5ab38 - languageName: node - linkType: hard - "math-interval-parser@npm:^2.0.1": version: 2.0.1 resolution: "math-interval-parser@npm:2.0.1" @@ -15762,13 +15730,6 @@ __metadata: languageName: node linkType: hard -"sprintf-js@npm:~1.0.2": - version: 1.0.3 - resolution: "sprintf-js@npm:1.0.3" - checksum: 10/c34828732ab8509c2741e5fd1af6b767c3daf2c642f267788f933a65b1614943c282e74c4284f4fa749c264b18ee016a0d37a3e5b73aee446da46277d3a85daa - languageName: node - linkType: hard - "sql-escaper@npm:^1.3.3": version: 1.3.3 resolution: "sql-escaper@npm:1.3.3"