Skip to content

fix(security): 2 improvements across 2 files#1651

Open
tomaioo wants to merge 2 commits into
floatpane:masterfrom
tomaioo:fix/security/unsafe-html-rendering-in-markdown-fallba
Open

fix(security): 2 improvements across 2 files#1651
tomaioo wants to merge 2 commits into
floatpane:masterfrom
tomaioo:fix/security/unsafe-html-rendering-in-markdown-fallba

Conversation

@tomaioo

@tomaioo tomaioo commented Jul 5, 2026

Copy link
Copy Markdown

Summary

fix(security): 2 improvements across 2 files

Problem

Severity: High | File: clib/markdown_nocgo.go:L17

The markdownPlainTextHTML function in clib/markdown_fallback.go wraps markdown content in a <pre> tag after escaping with html.EscapeString. While this escapes standard HTML entities, the function name and approach suggest it's a fallback for markdown rendering. The html.EscapeString function properly escapes <, >, &, and quotes, so this is actually safe for the intended purpose. However, the MarkdownToHTML function in clib/markdown_nocgo.go uses html.WithUnsafe() renderer option with goldmark, which allows raw HTML in the markdown source to be rendered unescaped. This is a more significant concern as it could lead to XSS if user-controlled markdown is processed.

Solution

Remove html.WithUnsafe() from the goldmark renderer options, or ensure that all markdown input is properly sanitized before processing. The WithUnsafe option allows raw HTML to pass through, which is dangerous for untrusted content.

Changes

  • clib/markdown_nocgo.go (modified)
  • notify/notify.go (modified)

tomaioo added 2 commits July 4, 2026 23:21
- Security: Unsafe HTML Rendering in Markdown Fallback
- Security: Command Injection via Unsanitized Input in Notification Function

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
- Security: Unsafe HTML Rendering in Markdown Fallback
- Security: Command Injection via Unsanitized Input in Notification Function

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
@tomaioo tomaioo requested a review from a team as a code owner July 5, 2026 06:21

@floatpanebot floatpanebot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tomaioo! Please fix the following issues with your PR:

  • Title: Is too long (44 characters). The PR title must be strictly under 40 characters.
  • Body: Missing the ## What? or ## Why? headings required by the PR template.

@floatpanebot floatpanebot added bug Something isn't working area/notify Notifications area/build Build system / Makefile / packaging size/XS Diff: 0–10 lines labels Jul 5, 2026
@floatpanebot

Copy link
Copy Markdown
Member

Benchmark report — no significant change

Metrics worse: 0 · better: 0 (threshold: ±3%).

benchstat output
goos: linux
goarch: amd64
pkg: github.com/floatpane/matcha/backend
cpu: AMD EPYC 7763 64-Core Processor                
                           │    old.txt    │               new.txt                │
                           │    sec/op     │    sec/op      vs base               │
ParseSearchQuery_Simple-4    2.956µ ± 400%    2.787µ ± 34%        ~ (p=0.394 n=6)
ParseSearchQuery_Complex-4   7.611µ ± 113%   10.532µ ± 47%        ~ (p=0.394 n=6)
TokenizeSearchQuery-4        3.953µ ±  30%    4.802µ ±  9%  +21.49% (p=0.041 n=6)
geomean                      4.463µ           5.204µ        +16.59%

                           │  old.txt   │              new.txt               │
                           │    B/op    │    B/op     vs base                │
ParseSearchQuery_Simple-4    26.00 ± 0%   26.00 ± 0%       ~ (p=1.000 n=6) ¹
ParseSearchQuery_Complex-4   762.0 ± 0%   762.0 ± 0%       ~ (p=1.000 n=6) ¹
TokenizeSearchQuery-4        176.0 ± 0%   176.0 ± 0%       ~ (p=1.000 n=6) ¹
geomean                      151.6        151.6       +0.00%
¹ all samples are equal

                           │  old.txt   │              new.txt               │
                           │ allocs/op  │ allocs/op   vs base                │
ParseSearchQuery_Simple-4    2.000 ± 0%   2.000 ± 0%       ~ (p=1.000 n=6) ¹
ParseSearchQuery_Complex-4   23.00 ± 0%   23.00 ± 0%       ~ (p=1.000 n=6) ¹
TokenizeSearchQuery-4        9.000 ± 0%   9.000 ± 0%       ~ (p=1.000 n=6) ¹
geomean                      7.453        7.453       +0.00%
¹ all samples are equal

pkg: github.com/floatpane/matcha/tui
                    │   old.txt    │               new.txt               │
                    │    sec/op    │    sec/op      vs base              │
LogPanelView-4        160.8µ ± 20%    165.3µ ± 21%       ~ (p=0.589 n=6)
SearchOverlayView-4   175.9µ ± 14%    179.3µ ± 10%       ~ (p=0.937 n=6)
InboxConstruction-4   928.7µ ±  7%   1021.1µ ± 20%       ~ (p=0.093 n=6)
geomean               297.3µ          311.6µ        +4.84%

                    │    old.txt    │               new.txt               │
                    │     B/op      │     B/op       vs base              │
LogPanelView-4        44.67Ki ± 51%   44.67Ki ± 51%       ~ (p=1.000 n=6)
SearchOverlayView-4   56.14Ki ± 41%   56.14Ki ± 41%       ~ (p=0.758 n=6)
InboxConstruction-4   874.2Ki ±  0%   874.3Ki ±  0%       ~ (p=0.558 n=6)
geomean               129.9Ki         129.9Ki        +0.00%

                    │   old.txt   │              new.txt              │
                    │  allocs/op  │  allocs/op   vs base              │
LogPanelView-4         714.0 ± 0%    714.0 ± 0%       ~ (p=1.000 n=6)
SearchOverlayView-4    926.0 ± 0%    926.0 ± 0%       ~ (p=0.636 n=6)
InboxConstruction-4   3.478k ± 0%   3.478k ± 0%       ~ (p=0.864 n=6)
geomean               1.320k        1.320k       +0.00%

auto-generated by benchmarks.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Build system / Makefile / packaging area/notify Notifications bug Something isn't working size/XS Diff: 0–10 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants