diff options
| -rw-r--r-- | src/markdownrenderer.cpp | 21 | ||||
| -rw-r--r-- | tests/test_markdownrenderer.cpp | 29 |
2 files changed, 43 insertions, 7 deletions
diff --git a/src/markdownrenderer.cpp b/src/markdownrenderer.cpp index a9b8177..ccb3309 100644 --- a/src/markdownrenderer.cpp +++ b/src/markdownrenderer.cpp @@ -34,8 +34,8 @@ namespace { /// /// `table` is absent deliberately, not by oversight: tables render badly /// across mail clients regardless of who generates them. `tagfilter` is absent -/// because CMARK_OPT_SAFE already suppresses raw HTML wholesale, which is the -/// stronger measure. +/// because safe mode (see below) already suppresses raw HTML wholesale, which +/// is the stronger measure. const char *const kExtensions[] = { "autolink", "strikethrough", "tasklist" }; } // namespace @@ -51,8 +51,21 @@ QString MarkdownRenderer::toHtml(const QString &markdown) // after the first call. cmark_gfm_core_extensions_ensure_registered(); - // SAFE suppresses raw HTML in the INPUT. It does not escape the output, - // which is markup by definition. + // CMARK_OPT_DEFAULT is 0, and CMARK_OPT_SAFE is a NO-OP in cmark-gfm 0.29: + // safe mode has been the default since that release, and the flag is kept + // only for API compatibility with code written against older versions. + // The real requirement is that CMARK_OPT_UNSAFE must never be set. Under + // safe mode a raw <script> block is replaced with an HTML comment + // placeholder, and a link whose scheme is not in the allowed set + // (javascript:, vbscript:, file:, and data: except a few safe image + // types) is replaced with an empty href. Measured against + // cmark-gfm-0.29.0.gfm.13 on 2026-08-20: rendering the same script tag and + // a javascript: link under OPT_DEFAULT alone, under OPT_DEFAULT|OPT_SAFE, + // and under OPT_UNSAFE shows the first two behave identically and + // suppress both, while OPT_UNSAFE leaks both verbatim into the output. + // OPT_SAFE is kept anyway, both as a statement of intent and in case a + // future cmark-gfm release makes it meaningful again; do not read its + // presence as the mechanism actually doing the suppressing. const int options = CMARK_OPT_DEFAULT | CMARK_OPT_SAFE; cmark_parser *parser = cmark_parser_new(options); diff --git a/tests/test_markdownrenderer.cpp b/tests/test_markdownrenderer.cpp index 44f3ccb..0c35b6c 100644 --- a/tests/test_markdownrenderer.cpp +++ b/tests/test_markdownrenderer.cpp @@ -35,6 +35,7 @@ private slots: void tasklistRenders(); void tablesAreNotEnabled(); void rawHtmlIsSuppressed(); + void unsafeLinksAreStripped(); void accentedTextSurvivesAsUtf8(); void emptyInputProducesEmptyOutput(); }; @@ -90,15 +91,37 @@ void TestMarkdownRenderer::tablesAreNotEnabled() void TestMarkdownRenderer::rawHtmlIsSuppressed() { - // CMARK_OPT_SAFE. The body is the user's own text, but a body that can - // inject markup into its own generated HTML part is a sharp edge with no - // upside. + // Safe mode (the cmark-gfm 0.29 default, not CMARK_OPT_SAFE, which is a + // no-op in this version, see markdownrenderer.cpp). The body is the + // user's own text, but a body that can inject markup into its own + // generated HTML part is a sharp edge with no upside. + // + // Asserted on the actual placeholder rather than only "no <script>", + // because the weaker assertion would still pass with CMARK_OPT_UNSAFE + // set by mistake, as long as something ELSE in the string also matched + // "not <script>" and "contains after" (measured: it does not distinguish + // safe from unsafe mode on its own). "raw HTML omitted" is what safe mode + // actually emits in place of the tag. const QString html = MarkdownRenderer::toHtml( QStringLiteral("<script>alert(1)</script>\n\nafter")); QVERIFY2(!html.contains(QStringLiteral("<script>")), qPrintable(html)); + QVERIFY2(html.contains(QStringLiteral("raw HTML omitted")), qPrintable(html)); QVERIFY2(html.contains(QStringLiteral("after")), qPrintable(html)); } +void TestMarkdownRenderer::unsafeLinksAreStripped() +{ + // A protection this gets for free from safe mode, and previously + // asserted nothing about: a javascript: link is replaced with an empty + // href rather than passed through. The body is the user's own text, but + // it is rendered into an HTML part sent to other people, so a + // javascript: link surviving into that part would be a real defect, not + // a cosmetic one. + const QString html = MarkdownRenderer::toHtml( + QStringLiteral("[click](javascript:alert(1))")); + QVERIFY2(!html.contains(QStringLiteral("javascript:")), qPrintable(html)); +} + void TestMarkdownRenderer::accentedTextSurvivesAsUtf8() { // This user writes Italian, so accented text is every message rather |