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
Contributor

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!

@floatpanebot floatpanebot added the stale No activity for extended period label Sep 4, 2026
@floatpanebot

Copy link
Copy Markdown
Member

This has had no activity for 45 days. It will be closed in 21 days unless updated. Comment or remove the stale label to keep it open.

@kawacukennedy

Copy link
Copy Markdown
Contributor Author

Hi @andrinoff — this PR is still open and all CI checks are green; just keeping it off the stale queue. Happy to rebase/adjust if anything needs updating. Thanks!

@floatpanebot floatpanebot removed the stale No activity for extended period label Sep 5, 2026
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