Skip to content

fix(view): block SSRF in remote images#1693

Open
kawacukennedy wants to merge 2 commits into
floatpane:masterfrom
kawacukennedy:fix/view-html-ssrf
Open

fix(view): block SSRF in remote images#1693
kawacukennedy wants to merge 2 commits into
floatpane:masterfrom
kawacukennedy:fix/view-html-ssrf

Conversation

@kawacukennedy

@kawacukennedy kawacukennedy commented Jul 20, 2026

Copy link
Copy Markdown

What?

Block remote image fetches to private/internal IP addresses and cap redirect chains to 5 hops. Reuse a single HTTP client per image fetch instead of creating one per call.

Why?

A malicious HTML email could embed images pointing at internal services (e.g. cloud metadata at 169.254.169.254, or 127.0.0.1). Viewing such an email triggered an outbound request to the attacker-controlled URL, enabling SSRF.

The default HTTP client also followed up to 10 redirects, allowing SSRF via redirect chains.

@kawacukennedy
kawacukennedy requested a review from a team as a code owner July 20, 2026 01:03
@floatpanebot floatpanebot added area/tui Terminal UI / view layer size/M Diff: 51–200 lines and removed area/tui Terminal UI / view layer labels Jul 20, 2026
…lient

A malicious HTML email could embed images pointing at internal services
(e.g. http://169.254.169.254/latest/meta-data/ on cloud providers, or
http://127.0.0.1:* to reach local applications). Every recipient who
views such an email triggers a request to the attacker-controlled URL,
enabling server-side request forgery.

Add isPrivateHost() DNS resolution check that rejects connections to
loopback (127.0.0.0/8), RFC 1918 (10/172.16-31/192.168), link-local
(169.254.0.0/16), and IPv6 loopback/link-local addresses.

Also limit redirect chains to 5 hops using httpclient.NewWithRedirectCap
to prevent SSRF via open-redirect chains, and reuse a single HTTP client
across calls instead of creating a new one per image fetch.

@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 @kawacukennedy! Please fix the following issues with your PR:

  • Title: Is too long (55 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 the area/tui Terminal UI / view layer label Jul 20, 2026
@floatpanebot

floatpanebot commented Jul 20, 2026

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 9V74 80-Core Processor                
                           │    old.txt    │              new.txt               │
                           │    sec/op     │    sec/op     vs base              │
ParseSearchQuery_Simple-4    2.607µ ± 311%   2.308µ ± 39%       ~ (p=0.394 n=6)
ParseSearchQuery_Complex-4   8.145µ ±  40%   8.445µ ± 87%       ~ (p=0.818 n=6)
TokenizeSearchQuery-4        4.345µ ±  40%   5.263µ ± 35%       ~ (p=0.180 n=6)
geomean                      4.519µ          4.681µ        +3.60%

                           │  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        158.6µ ± 10%    168.3µ ± 17%        ~ (p=0.180 n=6)
SearchOverlayView-4   163.9µ ±  8%    181.0µ ± 21%  +10.48% (p=0.009 n=6)
InboxConstruction-4   970.4µ ± 16%   1011.2µ ±  6%        ~ (p=0.093 n=6)
geomean               293.3µ          313.5µ         +6.91%

                    │    old.txt    │               new.txt               │
                    │     B/op      │     B/op       vs base              │
LogPanelView-4        33.23Ki ± 34%   33.23Ki ± 34%       ~ (p=1.000 n=6)
SearchOverlayView-4   33.17Ki ± 69%   33.19Ki ± 69%       ~ (p=0.784 n=6)
InboxConstruction-4   874.3Ki ±  0%   874.3Ki ±  0%       ~ (p=0.818 n=6)
geomean               98.78Ki         98.79Ki        +0.02%

                    │   old.txt   │              new.txt              │
                    │  allocs/op  │  allocs/op   vs base              │
LogPanelView-4         713.0 ± 0%    713.0 ± 0%       ~ (p=1.000 n=6)
SearchOverlayView-4    923.5 ± 0%    924.0 ± 0%       ~ (p=0.913 n=6)
InboxConstruction-4   3.478k ± 0%   3.478k ± 0%       ~ (p=0.634 n=6)
geomean               1.318k        1.318k       +0.02%

auto-generated by benchmarks.yml

@kawacukennedy kawacukennedy changed the title fix(view): add SSRF protection to remote image fetching fix(view): block SSRF in remote images Jul 20, 2026
@floatpanebot
floatpanebot dismissed their stale review July 20, 2026 01:33

Formatting issues have been resolved. Thank you!

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

Labels

area/tui Terminal UI / view layer size/M Diff: 51–200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants