Skip to content

Trim trailing dot from trusted IMDS hostnames #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bitterpanda63 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Trim trailing dot from trusted IMDS hostnames #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9c9a36
Trim trailing dot and normalize case for trusted IMDS hostnames
bitterpanda63 Jun 8, 2026
b789337
Reuse NormalizeHostname helper; add trailing-dot stripping to it
bitterpanda63 Jun 8, 2026
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
7 changes: 5 additions & 2 deletions lib/request-processor/helpers/normalizeHostname.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ func NormalizeHostname(hostname string) string {
// Convert to lowercase first - idna.ToUnicode requires lowercase "xn--" prefix
lowercased := strings.ToLower(decoded)

// Strip trailing dot — DNS resolvers may return FQDNs like "metadata.google.internal."
noDot := strings.TrimSuffix(lowercased, ".")

// Convert Punycode (xn--...) to Unicode form for consistent comparison
// e.g., "xn--mnchen-3ya.de" -> "münchen.de"
unicodeHostname, err := idna.ToUnicode(lowercased)
unicodeHostname, err := idna.ToUnicode(noDot)
if err != nil {
// If conversion fails, use the lowercased hostname as-is
unicodeHostname = lowercased
unicodeHostname = noDot
}

return unicodeHostname
Expand Down
10 changes: 10 additions & 0 deletions lib/request-processor/helpers/normalizeHostname_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ func TestNormalizeHostname(t *testing.T) {
input: "example.com\u200B",
expected: "example.com",
},
{
name: "FQDN with trailing dot",
input: "example.com.",
expected: "example.com",
},
{
name: "GCP IMDS hostname with trailing dot",
input: "metadata.google.internal.",
expected: "metadata.google.internal",
},
}

for _, tt := range tests {
Expand Down
5 changes: 3 additions & 2 deletions lib/request-processor/vulnerabilities/ssrf/imds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package ssrf

import "main/helpers"

// This IP address is used by AWS EC2 instances to access the instance metadata service (IMDS)
// We should block any requests to these IP addresses
// This prevents STORED SSRF attacks that try to access the instance metadata service
Expand All @@ -24,8 +26,7 @@ func isIMDSIPAddress(ip string) bool {
}

func isTrustedHostname(hostname string) bool {
// Check if the hostname is in the trusted hosts map
_, exists := trustedHosts[hostname]
_, exists := trustedHosts[helpers.NormalizeHostname(hostname)]

@aikido-pr-checks aikido-pr-checks Bot Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Replacing the direct trustedHosts map lookup with helpers.NormalizeHostname(hostname) adds expensive work (URL unescape + IDN conversion) on a likely hot path; consider pre-normalizing or caching hostnames.

Suggested change
_, exists := trustedHosts[helpers.NormalizeHostname(hostname)]
_, exists := trustedHosts[hostname]
Details

✨ AI Reasoning
​The code previously did a direct map lookup of the hostname (cheap O(1)). The diff replaces that with helpers.NormalizeHostname(hostname), which runs TrimInvisible, url.QueryUnescape, strings.ToLower, and idna.ToUnicode. These operations allocate and perform non-trivial work (especially IDN conversion). Because isTrustedHostname is likely called per-request for each resolved host, this introduces a performance regression by doing expensive normalization on every check. The added TrimSuffix itself is cheap, but the overall NormalizeHostname call makes the previously trivial lookup much heavier. A lightweight alternative would be to normalize trustedHosts keys up-front or cache normalization results.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

return exists
}

Expand Down
Loading