Icons service drops non-default ports, so self-hosted sites on custom ports get the wrong favicon
- Dominant language
- C#
- Stars
- 20.1k
- Forks
- 1.8k
- Avg merge
- 4d 8h
- Merged PRs (30d)
- 75
Description
### Summary
On a self-hosted instance, a login item whose URI uses a non-default port (for example `https://example.tld:2053/`) never gets the right favicon. The Icons service strips the port and fetches the default port (443/80) instead. When a different site answers on that host's default port (a catch-all vhost, or another app on the same host), the Icons service returns that other site's favicon. The item then shows an unrelated logo instead of no icon at all, and a wrong icon is more confusing than a blank one.
This was reported before in #1010 and closed as not planned with no technical discussion. This issue adds the code locations, the security history, and a concrete opt-in proposal.
### Environment
- Self-hosted server (Icons container).
- Any client (web, desktop, browser extension, mobile), since they all render whatever the Icons service returns.
- Example: vault item URI `https://example.tld:2053/`, match detection Exact. The host answers Bitwarden on `:2053`, and a different site answers on `:443`. The item shows the `:443` site's favicon.
### Root cause
The port is discarded in two independent places.
Client (context, in `bitwarden/clients`): the icon URL is built from the hostname only. `libs/common/src/vault/icon/build-cipher-icon.ts` uses `Utils.getHostname()` (the tldts library), which returns `example.tld` without the port. The request that leaves the client is already `//example.tld/icon.png`, so the port never travels to the server.
Server (`bitwarden/server`):
- `src/Icons/Controllers/IconsController.cs` reads `var domain = uri.Host;`, which drops any port that did arrive.
- `src/Icons/Models/IconUri.cs` rejects non-default ports in `IsValid`: `(InnerUri.Scheme != "http" && InnerUri.Scheme != "https") || !InnerUri.IsDefaultPort`.
- Even if the port reached this point, other rebuilders drop it too: `IconFetchingService.GetFaviconAsync` (a `UriBuilder` with `Host` only), `DomainIcons.FetchIconsAsync` (`$"https://{Domain}"` and then `uri.Host` for the www fallback), and `UriExtension.ChangeScheme` (a new `UriBuilder` without the port).
- The response cache is keyed by hostname (`mappedDomain` in `IconsController`), so a port-aware fix needs the port in the cache key as well.
### Why it currently works this way
The default-port restriction is deliberate SSRF hardening, not an oversight. It was added on 2020-04-30 in two commits (cc @kspearrin):
- `68901437b` "only fetch icons from http(s) with default ports"
- `3462613f4` "Do not request local hosts or ip addresses"
The 2023 rewrite (#3023) kept the behavior and moved the check into `IconUri.IsValid`. The Icons service makes outbound GET requests to user-controlled hostnames, so restricting it to http(s), default ports, and non-internal addresses limits its use as an SSRF probe. This is the same class of issue as the disclosed HackerOne report #925527 ("Blind HTTP GET SSRF via website icons"). Any change here has to keep that protection intact for the hosted service.
### Proposed solution
Make port support an explicit, self-hosted opt-in that is off by default, so the hosted `icons.bitwarden.net` service is unaffected.
Server:
1. Add a flag to `IconsSettings` (for example `AllowNonStandardPorts`, default false), configured the same way as the existing `CacheEnabled` and `CacheHours` settings (appsettings or environment variable).
2. When the flag is off, behavior is unchanged. When it is on, `IconUri.IsValid` allows a non-default port while keeping every other guard: http/https only, no IP literals, and no internal or local hosts.
3. Preserve the port through the pipeline: keep it in `IconsController` (use the authority instead of `uri.Host`), in `GetFaviconAsync`, in `DomainIcons`, and in `ChangeScheme`. `IconUri` already preserves the port when it swaps the host for the resolved IP, because `UriBuilder` copies it.
4. Include the port in the response cache key.
Client (`bitwarden/clients`, required for any visible effect): send the host with its port to the Icons service, so the request becomes `//example.tld:2053/icon.png`.
The server flag alone changes nothing for users, because the port never reaches the server today. The client has to send the port as well, which makes this a cross-repo change. The clients monorepo covers web, desktop, browser extension, and CLI in one place (`build-cipher-icon.ts`). Native iOS (`bitwarden/ios`) and Android (`bitwarden/android`) build the icon URL separately and would each need their own change.
### Offer to implement
I am happy to pick up the development. I can do the server change and the clients monorepo change behind the opt-in flag. I cannot test every client combination, in particular native iOS and Android, so I would need guidance on the scope you would accept and help with QA for full client coverage. One option is to land the server flag plus the monorepo clients first, with mobile as a follow-up.
Before writing code, I would like maintainer feedback on two points:
1. Is a self-hosted opt-in flag an acceptable way to support this, given the SSRF history?
2. Which clients do you want covered in the first pass?
### References
- Prior report: #1010 (closed as not planned)
- Rewrite that relocated the check: #3023
- Commits that introduced the restriction: `68901437b`, `3462613f4`
- SSRF background: HackerOne #925527
- Icons privacy model: https://bitwarden.com/help/website-icons/
Contributor guide
Research direction
Start with src/Icons/Controllers/IconsController.cs, src/Icons/Models/IconUri.cs, IconFetchingService.GetFaviconAsync, DomainIcons.FetchIconsAsync, UriExtension.ChangeScheme, and libs/common/src/vault/icon/build-cipher-icon.ts. Review the SSRF-related commits and confirm the opt-in scope with maintainers before changing code. Done means non-default ports survive the client and server pipeline and cache key while all existing SSRF guards remain active when the flag is off.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, typescript
- Domain
- api, backend, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100