Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion cli/config/credentials/file_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (c *fileStore) Store(authConfig types.AuthConfig) error {
// stored as hostname or as hostname including scheme (in legacy configuration
// files).
//
// It's the equivalent to [registry.ConvertToHostname] in the daemon.
// It's based on [registry.ConvertToHostname] from Moby daemon.
//
// [registry.ConvertToHostname]: https://pkg.go.dev/github.com/moby/moby/v2@v2.0.0-beta.7/daemon/pkg/registry#ConvertToHostname
func ConvertToHostname(maybeURL string) string {
Expand All @@ -117,7 +117,48 @@ func ConvertToHostname(maybeURL string) string {
}
return net.JoinHostPort(u.Hostname(), u.Port())
}

if hostName := hostFromURLFallback(stripped); hostName != "" {
return hostName
}
}
hostName, _, _ := strings.Cut(stripped, "/")
return hostName
}

// hostFromURLFallback extracts a host from scheme URLs that net/url rejects.
// Go rejects unbracketed IPv6 literals in URL hosts since
// https://github.com/golang/go/commit/0c28789bd7dfc55099cac86a3212dda0d6c091f6
func hostFromURLFallback(maybeURL string) string {
_, rest, ok := strings.Cut(maybeURL, "://")
if !ok {
return ""
}

hostName, _, _ := strings.Cut(rest, "/")
if hostName == "" {
return ""
}

if strings.Count(hostName, ":") > 1 && !strings.HasPrefix(hostName, "[") {
portStart := strings.LastIndex(hostName, ":")
addr, port := hostName[:portStart], hostName[portStart+1:]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MEDIUM] hostFromURLFallback misidentifies last IPv6 segment as port, producing a malformed address

For any unbracketed IPv6 address without an explicit port — e.g. a legacy credential stored as "https://2001:db8::1/path" — the fallback parser splits on the last : and misidentifies the final group of the IPv6 address as a port number.

Trace for "https://2001:db8::1/path":

  1. url.Parse fails (Go 1.26 rejects unbracketed IPv6), so the fallback is invoked.
  2. After stripping the scheme: hostName = "2001:db8::1".
  3. strings.Count("2001:db8::1", ":") = 3 > 1 → enters the branch.
  4. portStart = strings.LastIndex("2001:db8::1", ":") = 9addr = "2001:db8:", port = "1".
  5. isPort("1") = true → returns net.JoinHostPort("2001:db8:", "1") = "[2001:db8:]:1".

The addr value "2001:db8:" has a trailing colon and is not a valid IPv6 address. Any credential previously stored under "https://2001:db8::1" would fail to match after normalization because the key becomes "[2001:db8:]:1" instead of "2001:db8::1".

Fix suggestion: Before splitting on the last :, check whether the suffix after the last : is also a valid hex IPv6 group — or, more robustly, attempt net.ParseIP(hostName) first. If it parses as a valid IP (no port), return net.JoinHostPort(hostName, "") (i.e. bracket but no port) or just hostName unchanged.

if addr != "" && isPort(port) {
return net.JoinHostPort(addr, port)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[LOW] Non-numeric-port branch falls through and returns unbracketed IPv6 string

When strings.Count(hostName, ":") > 1 (IPv6-like) but the last segment is not a decimal port (e.g. "https://::1:bad/"), the if addr != "" && isPort(port) guard fails and the function falls through to return hostName, returning "::1:bad" — an unbracketed IPv6 string — rather than a bracketed form.

All other code paths that handle IPv6 addresses produce bracketed output (net.JoinHostPort always adds brackets). This inconsistency could surprise callers that assume IPv6 addresses are always returned in [addr] or [addr]:port form.

In practice, credentials with non-numeric ports are not realistic, so the real-world impact is near zero. But the fall-through behaviour is worth documenting (or guarding) for correctness.

}

return hostName
}

func isPort(port string) bool {
if port == "" {
return false
}
for _, r := range port {
if r < '0' || r > '9' {
return false
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[LOW] isPort accepts values > 65535 — no upper-bound validation

isPort validates only that all characters are ASCII digits; it does not check that the parsed number is ≤ 65535. An input like "https://::1:99999/" passes the check and returns "[::1]:99999" as the normalised credential key.

In practice this mirrors whatever was stored under old Go (which also had no port-range enforcement in url.Parse), so the risk of a new mismatch is low. Nonetheless, adding a simple range check (strconv.Atoi + n >= 0 && n <= 65535) would make the validation more correct and align with the semantics of a real TCP port.

return true
}
Loading