Skip to content

feat: add auto wildcard detection and filtering#948

Open
Pitrat-wav wants to merge 2 commits intoprojectdiscovery:devfrom
Pitrat-wav:dev
Open

feat: add auto wildcard detection and filtering#948
Pitrat-wav wants to merge 2 commits intoprojectdiscovery:devfrom
Pitrat-wav:dev

Conversation

@Pitrat-wav
Copy link

@Pitrat-wav Pitrat-wav commented Feb 23, 2026

Description

This PR implements automatic wildcard DNS detection and filtering, as requested in issue #924.

Changes

  • New Flag: Added --auto-wildcard (-aw) flag to automatically detect and filter wildcard DNS records across multiple domains
  • Wildcard Detection: Implemented IsWildcardDomain() to detect wildcard domains by querying random subdomains
  • Host-level Checking: Implemented IsWildcardAuto() to check if a specific host is a wildcard
  • Base Domain Extraction: Added GetBaseDomain() helper to extract base domain from subdomain
  • Auto Filtering: Added runAutoWildcardFiltering() to process all domains with auto wildcard detection
  • Caching: Results are cached per domain for efficiency
  • Validation: Added validation to prevent using --wildcard-domain and --auto-wildcard together
  • Stream Mode: Auto wildcard mode is not supported in stream mode (consistent with existing wildcard behavior)

Usage

# Auto detect and filter wildcards across multiple domains
echo -e "sub1.example.com\nsub2.example.com\ntest.test.com" | dnsx -aw

# Or with a file
dnsx -l subdomains.txt -aw

Testing

  • Build passes without errors
  • Existing tests pass (TestRunner_domainWildCard_prepareInput)
  • Manual testing confirms wildcard detection works correctly

Related

Summary by CodeRabbit

  • New Features

    • Added automatic wildcard domain detection via --auto-wildcard / -aw.
    • Automatically identifies and removes wildcard subdomains from results, reporting how many were filtered.
    • Improved domain analysis to better group and evaluate hosts.
  • Bug Fixes

    • Validation prevents using auto-wildcard with explicit wildcard-domain simultaneously.
    • Auto-wildcard option disabled in stream mode to avoid unsupported runtime behavior.

- Add --auto-wildcard (-aw) flag to automatically detect and filter wildcard DNS records
- Implement IsWildcardDomain() to detect wildcard domains by querying random subdomains
- Implement IsWildcardAuto() to check if a specific host is a wildcard
- Add GetBaseDomain() helper to extract base domain from subdomain
- Add runAutoWildcardFiltering() to process all domains with auto wildcard detection
- Cache wildcard detection results per domain for efficiency
- Add validation to prevent using --wildcard-domain and --auto-wildcard together
- Auto wildcard mode not supported in stream mode

Fixes: projectdiscovery#924

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@neo-by-projectdiscovery-dev
Copy link

neo-by-projectdiscovery-dev bot commented Feb 23, 2026

Neo - PR Security Review

No security issues found

Highlights

  • Added publicsuffix-go library (v0.40.3) to properly handle multi-level TLDs like .co.uk and .com.au
  • Enhanced GetBaseDomain() function with publicsuffix.Parse() and safe fallback logic
  • Implementation includes proper error handling and validation before using parsed domain data

Comment @neo help for available commands. · Open in Neo

@coderabbitai
Copy link

coderabbitai bot commented Feb 23, 2026

Walkthrough

Adds an optional AutoWildcard mode that detects and filters wildcard DNS across multiple domains at runtime, with CLI flag support, new runner orchestration for parallel detection, and helper methods for domain and host wildcard checks with caching.

Changes

Cohort / File(s) Summary
Configuration
internal/runner/options.go
Adds AutoWildcard bool to Options, CLI flags --auto-wildcard/-aw, validation to forbid combining with wildcard-domain and to disallow in stream mode.
Core runner orchestration
internal/runner/runner.go
Initializes autoWildcardDomains cache, runs runAutoWildcardFiltering when enabled, spawns wildcardWorkerAuto workers, integrates auto-wildcard flow into existing output/restart logic, and counts/filters wildcard subdomains.
Wildcard detection helpers
internal/runner/wildcard.go
Adds GetBaseDomain, IsWildcardDomain, and IsWildcardAuto methods; introduces publicsuffix usage and per-domain/host caching and DNS comparison logic for auto-detection.

Sequence Diagram

sequenceDiagram
    participant User
    participant Runner
    participant Resolver as DNS Resolver/Cache
    participant WorkerPool as Auto-Wildcard Workers
    participant Output

    User->>Runner: Start with --auto-wildcard
    Runner->>Runner: Scan stored DNS records\nGroup hosts by base domain\nBuild IP->hosts mappings
    Runner->>WorkerPool: Dispatch hosts for IsWildcardAuto checks
    WorkerPool->>Resolver: Query base domain and random subdomains\nCompare A records
    Resolver-->>WorkerPool: Return cached or queried results
    WorkerPool-->>Runner: Report wildcard hosts
    Runner->>Runner: Filter wildcard hosts\nRestart/merge output workflow
    Runner->>Output: Emit non-wildcard results
    Output-->>User: Filtered DNS results
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I sniff the DNS breeze at night,

hopping through domains till daylight,
spawning workers, checking each name,
hiding wildcards from the fame—
auto-wildcard, hop! — the list is tame.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main feature addition: auto wildcard detection and filtering implementation.
Linked Issues check ✅ Passed All coding objectives from issue #924 are met: auto wildcard detection across multiple domains, automatic filtering, and simple flag exposure (--auto-wildcard).
Out of Scope Changes check ✅ Passed All changes are aligned with the scope of issue #924; the publicsuffix library addition for accurate base domain extraction is a necessary supporting change for the core feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
internal/runner/runner.go (1)

582-614: ipDomain and listIPs are computed but unused in auto-wildcard filtering.

These structures mirror the WildcardDomain path (which uses IP-frequency thresholds), but the auto path doesn't use them beyond a debug log message (Line 615). The auto path sends all hosts to workers unconditionally. Consider removing ipDomain and listIPs to avoid confusion and unnecessary allocations, or if IP-based grouping is intended for future use, add a comment explaining why.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/runner.go` around lines 582 - 614, The ipDomain and listIPs
variables are allocated and populated inside runAutoWildcardFiltering (via
r.hm.Scan) but never used beyond a debug log, causing unnecessary allocations;
remove the ipDomain and listIPs declarations and the code that populates them
(the map creation, the listIPs append, and ipDomain[a][host] assignments) and
leave only domainHosts population, or alternatively keep ipDomain/listIPs but
add a clear TODO comment in runAutoWildcardFiltering explaining they are
intentionally reserved for future IP-frequency logic; adjust any debug/logging
to not reference listIPs when removed.
internal/runner/wildcard.go (1)

73-93: Single random-subdomain query may yield false positives.

IsWildcardDomain sends only one random subdomain query. While xid makes accidental collisions unlikely, DNS middleboxes or captive portals can resolve any query to a valid IP (e.g., some ISP resolvers). Querying 2–3 random subdomains and checking that they all resolve (ideally to the same IP set) would significantly reduce false positives.

This is how tools like puredns approach it and would be consistent with the PR's stated goal of similarity to puredns automatic detection.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/wildcard.go` around lines 73 - 93, IsWildcardDomain currently
queries only one random subdomain (using xid.New().String() and r.dnsx.QueryOne)
which can yield false positives from captive portals or DNS middleboxes; change
it to query 2–3 different random subdomains, collect their DNS responses via
r.dnsx.QueryOne (or Query if available), and only mark
r.autoWildcardDomains.Set(domain, true) if all queries resolve and their
IP/answer sets are consistent (or at least all resolve); otherwise set the cache
to false. Ensure you still check the cache at the start, generate distinct
random names for each probe, and short-circuit to false if any probe fails to
resolve.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/runner/runner.go`:
- Line 826: The code only forces A-record querying when r.options.WildcardDomain
is set but misses the same enforcement for r.options.AutoWildcard; update the
New function so that the conditional that checks r.options.WildcardDomain also
triggers when r.options.AutoWildcard is true (i.e., if r.options.WildcardDomain
!= "" || r.options.AutoWildcard) and then ensure the options are adjusted to
query A records (add/ensure 'A' record type is present and/or remove conflicting
types like 'AAAA') before wildcard detection runs (references: New function,
r.options.WildcardDomain, r.options.AutoWildcard, and the wildcard detection
helpers IsWildcardAuto/IsWildcardDomain).
- Around line 617-637: The loop that starts wildcard workers uses numThreads
derived from r.options.Threads and capped by len(listIPs), which can be zero
when listIPs has no A records, causing no goroutines to run while domainHosts
are later sent on r.wildcardworkerchan and deadlock; fix by basing numThreads on
the number of hosts (len(domainHosts) or total host count across domainHosts)
with a sensible max of r.options.Threads (or at least 1), ensure
r.wgwildcardworker.Add is called for that computed numThreads and that
r.wildcardWorkerAuto is launched that many times so consumers exist before
sending into r.wildcardworkerchan, and keep closing r.wildcardworkerchan after
all producers have sent.

In `@internal/runner/wildcard.go`:
- Around line 130-140: The loop building hosts in wildcard.go has an off-by-one:
change the loop condition in the block that iterates subdomainTokens (currently
using i <= len(subdomainTokens)) to stop before the slice becomes empty (use i <
len(subdomainTokens>) so that strings.Join(subdomainTokens[i:], ".") never
returns an empty string); update the loop that constructs newhost (the one
referencing subdomainTokens, baseDomain, and hosts) to match the logic used by
IsWildcard so it doesn't append a host like ".example.com".
- Around line 96-103: GetBaseDomain is incorrect for multi-part TLDs and
IsWildcardAuto has an off-by-one loop bug; fix GetBaseDomain by using a public
suffix parser (e.g., github.com/weppos/publicsuffix-go) to compute the
registrable domain (eTLD+1) in Runner.GetBaseDomain and fall back to joining the
last two labels only if the parser returns an error or nil rule, and fix
Runner.IsWildcardAuto by changing the loop bound from i <= len(subdomainTokens)
to i < len(subdomainTokens) so subdomainTokens[i:] never becomes empty (avoid
producing a newhost with a leading dot).

---

Nitpick comments:
In `@internal/runner/runner.go`:
- Around line 582-614: The ipDomain and listIPs variables are allocated and
populated inside runAutoWildcardFiltering (via r.hm.Scan) but never used beyond
a debug log, causing unnecessary allocations; remove the ipDomain and listIPs
declarations and the code that populates them (the map creation, the listIPs
append, and ipDomain[a][host] assignments) and leave only domainHosts
population, or alternatively keep ipDomain/listIPs but add a clear TODO comment
in runAutoWildcardFiltering explaining they are intentionally reserved for
future IP-frequency logic; adjust any debug/logging to not reference listIPs
when removed.

In `@internal/runner/wildcard.go`:
- Around line 73-93: IsWildcardDomain currently queries only one random
subdomain (using xid.New().String() and r.dnsx.QueryOne) which can yield false
positives from captive portals or DNS middleboxes; change it to query 2–3
different random subdomains, collect their DNS responses via r.dnsx.QueryOne (or
Query if available), and only mark r.autoWildcardDomains.Set(domain, true) if
all queries resolve and their IP/answer sets are consistent (or at least all
resolve); otherwise set the cache to false. Ensure you still check the cache at
the start, generate distinct random names for each probe, and short-circuit to
false if any probe fails to resolve.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe80b18 and 7ef3fa1.

📒 Files selected for processing (3)
  • internal/runner/options.go
  • internal/runner/runner.go
  • internal/runner/wildcard.go

Comment on lines +617 to +637
// Start wildcard workers for auto detection
numThreads := r.options.Threads
if numThreads > len(listIPs) {
numThreads = len(listIPs)
}
for i := 0; i < numThreads; i++ {
r.wgwildcardworker.Add(1)
go r.wildcardWorkerAuto()
}

// Send hosts to wildcard worker for checking
seen := make(map[string]struct{})
for _, hosts := range domainHosts {
for _, host := range hosts {
if _, ok := seen[host]; !ok {
seen[host] = struct{}{}
r.wildcardworkerchan <- host
}
}
}
close(r.wildcardworkerchan)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Potential deadlock: worker count is bounded by len(listIPs), but work is driven by domainHosts.

numThreads is capped at len(listIPs) (Line 619), but listIPs only contains A-record IPs. If all resolved hosts have only AAAA records (or other non-A types), listIPs is empty → numThreads == 0 → no workers started. The subsequent loop (Lines 629–636) then tries to send on the unbuffered wildcardworkerchan with no reader, causing a deadlock.

Even outside the deadlock scenario, bounding threads by IP count doesn't make sense for auto mode — the work unit is hosts, not IPs.

🐛 Proposed fix
 	// Start wildcard workers for auto detection
 	numThreads := r.options.Threads
-	if numThreads > len(listIPs) {
-		numThreads = len(listIPs)
+	totalHosts := 0
+	for _, hosts := range domainHosts {
+		totalHosts += len(hosts)
+	}
+	if numThreads > totalHosts {
+		numThreads = totalHosts
+	}
+	if numThreads == 0 {
+		return
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Start wildcard workers for auto detection
numThreads := r.options.Threads
if numThreads > len(listIPs) {
numThreads = len(listIPs)
}
for i := 0; i < numThreads; i++ {
r.wgwildcardworker.Add(1)
go r.wildcardWorkerAuto()
}
// Send hosts to wildcard worker for checking
seen := make(map[string]struct{})
for _, hosts := range domainHosts {
for _, host := range hosts {
if _, ok := seen[host]; !ok {
seen[host] = struct{}{}
r.wildcardworkerchan <- host
}
}
}
close(r.wildcardworkerchan)
// Start wildcard workers for auto detection
numThreads := r.options.Threads
totalHosts := 0
for _, hosts := range domainHosts {
totalHosts += len(hosts)
}
if numThreads > totalHosts {
numThreads = totalHosts
}
if numThreads == 0 {
return
}
// Send hosts to wildcard worker for checking
seen := make(map[string]struct{})
for _, hosts := range domainHosts {
for _, host := range hosts {
if _, ok := seen[host]; !ok {
seen[host] = struct{}{}
r.wildcardworkerchan <- host
}
}
}
close(r.wildcardworkerchan)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/runner.go` around lines 617 - 637, The loop that starts
wildcard workers uses numThreads derived from r.options.Threads and capped by
len(listIPs), which can be zero when listIPs has no A records, causing no
goroutines to run while domainHosts are later sent on r.wildcardworkerchan and
deadlock; fix by basing numThreads on the number of hosts (len(domainHosts) or
total host count across domainHosts) with a sensible max of r.options.Threads
(or at least 1), ensure r.wgwildcardworker.Add is called for that computed
numThreads and that r.wildcardWorkerAuto is launched that many times so
consumers exist before sending into r.wildcardworkerchan, and keep closing
r.wildcardworkerchan after all producers have sent.

}
// if wildcard filtering just store the data
if r.options.WildcardDomain != "" {
if r.options.WildcardDomain != "" || r.options.AutoWildcard {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

A-record query type is not enforced when AutoWildcard is enabled.

Line 119 ensures A records are queried when WildcardDomain is set, but the same guard is missing for AutoWildcard. The wildcard detection logic (IsWildcardAuto, IsWildcardDomain) exclusively inspects A records. If a user runs dnsx --aaaa --auto-wildcard, no A records are stored, and the wildcard detection is effectively a no-op.

🐛 Proposed fix (in the `New` function around line 119)
-	if len(questionTypes) == 0 || options.WildcardDomain != "" {
+	if len(questionTypes) == 0 || options.WildcardDomain != "" || options.AutoWildcard {
 		options.A = true
 		questionTypes = append(questionTypes, dns.TypeA)
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/runner.go` at line 826, The code only forces A-record
querying when r.options.WildcardDomain is set but misses the same enforcement
for r.options.AutoWildcard; update the New function so that the conditional that
checks r.options.WildcardDomain also triggers when r.options.AutoWildcard is
true (i.e., if r.options.WildcardDomain != "" || r.options.AutoWildcard) and
then ensure the options are adjusted to query A records (add/ensure 'A' record
type is present and/or remove conflicting types like 'AAAA') before wildcard
detection runs (references: New function, r.options.WildcardDomain,
r.options.AutoWildcard, and the wildcard detection helpers
IsWildcardAuto/IsWildcardDomain).

Comment on lines +96 to +103
func (r *Runner) GetBaseDomain(host string) string {
parts := strings.Split(host, ".")
if len(parts) <= 2 {
return host
}
// Return the last two parts as the base domain
return strings.Join(parts[len(parts)-2:], ".")
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find and read the wildcard.go file
find . -name "wildcard.go" -path "*/runner/*" -type f

Repository: projectdiscovery/dnsx

Length of output: 94


🏁 Script executed:

# Check the file exists and get its line count
wc -l internal/runner/wildcard.go 2>/dev/null || echo "File not found at expected path"

Repository: projectdiscovery/dnsx

Length of output: 96


🏁 Script executed:

# Read the entire file to verify the code and function implementations
cat -n internal/runner/wildcard.go

Repository: projectdiscovery/dnsx

Length of output: 5729


🏁 Script executed:

# Check publicsuffix dependency in go.mod
rg -A2 -B2 "publicsuffix" go.mod go.sum 2>/dev/null || echo "publicsuffix dependency not found"

Repository: projectdiscovery/dnsx

Length of output: 1251


🏁 Script executed:

# Also check what DNS/resolver libraries are imported in the wildcard.go file
rg "^import" -A20 internal/runner/wildcard.go

Repository: projectdiscovery/dnsx

Length of output: 538


🏁 Script executed:

# Check if publicsuffix-go is imported anywhere
rg "publicsuffix-go|weppos/publicsuffix" --type=go

Repository: projectdiscovery/dnsx

Length of output: 47


🏁 Script executed:

# Also check for publicsuffix imports
rg "import.*publicsuffix" --type=go

Repository: projectdiscovery/dnsx

Length of output: 47


🏁 Script executed:

# Check what publicsuffix variant is available
rg "golang.org/x/net/publicsuffix" --type=go

Repository: projectdiscovery/dnsx

Length of output: 47


🏁 Script executed:

# Check the documentation/usage of publicsuffix-go package
# First, find if there's any documentation or examples in go.mod comments
rg "weppos" go.mod go.sum

# Also search for any comment about the publicsuffix-go dependency
rg -B5 -A5 "publicsuffix-go" go.mod

Repository: projectdiscovery/dnsx

Length of output: 1239


🏁 Script executed:

# Let's check if there are any tests or examples using publicsuffix
find . -name "*.go" -type f -exec grep -l "publicsuffix\|publicSuffix" {} \;

Repository: projectdiscovery/dnsx

Length of output: 47


🏁 Script executed:

# Since publicsuffix-go is in the dependency, let's check what the actual API should be
# by looking at the common patterns in similar projects
# First, let me verify the exact import path and common function names
rg -n "github.com/weppos" . --type go 2>/dev/null | head -20

Repository: projectdiscovery/dnsx

Length of output: 47


Fix two critical bugs in GetBaseDomain and IsWildcardAuto loop bounds.

Issue 1: GetBaseDomain breaks for multi-part TLDs (lines 96-103)

Taking the last two labels fails for domains like sub.example.co.uk, returning co.uk instead of example.co.uk. This causes IsWildcardAuto to query wildcards against the TLD itself, producing incorrect results.

Issue 2: IsWildcardAuto off-by-one loop bug (line 134)

The loop uses i <= len(subdomainTokens) instead of i < len(subdomainTokens) (compare to IsWildcard line 33). When i == len(subdomainTokens), subdomainTokens[i:] returns an empty slice, and strings.Join([], ".") produces "", resulting in newhost = "." + baseDomain. This creates invalid DNS queries with a leading dot (e.g., .example.co.uk). The check on line 136 passes because the empty string is concatenated, not the result itself.

Suggested fixes

For GetBaseDomain, use a public suffix parser to correctly extract the registrable domain (eTLD+1):

import "github.com/weppos/publicsuffix-go/publicsuffix"

func (r *Runner) GetBaseDomain(host string) string {
	rule, err := publicsuffix.DefaultList.Find(host)
	if err != nil || rule == nil {
		// Fallback: return the last two parts
		parts := strings.Split(host, ".")
		if len(parts) <= 2 {
			return host
		}
		return strings.Join(parts[len(parts)-2:], ".")
	}
	return rule.Prevailing(host)
}

For IsWildcardAuto, change line 134 to use strict less-than:

for i := 1; i < len(subdomainTokens); i++ {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/wildcard.go` around lines 96 - 103, GetBaseDomain is
incorrect for multi-part TLDs and IsWildcardAuto has an off-by-one loop bug; fix
GetBaseDomain by using a public suffix parser (e.g.,
github.com/weppos/publicsuffix-go) to compute the registrable domain (eTLD+1) in
Runner.GetBaseDomain and fall back to joining the last two labels only if the
parser returns an error or nil rule, and fix Runner.IsWildcardAuto by changing
the loop bound from i <= len(subdomainTokens) to i < len(subdomainTokens) so
subdomainTokens[i:] never becomes empty (avoid producing a newhost with a
leading dot).

Comment on lines +130 to +140
var hosts []string
hosts = append(hosts, baseDomain)

if len(subdomainTokens) > 0 && subdomainTokens[0] != "" {
for i := 1; i <= len(subdomainTokens); i++ {
newhost := strings.Join(subdomainTokens[i:], ".") + "." + baseDomain
if newhost != "" {
hosts = append(hosts, newhost)
}
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Off-by-one in loop produces a host with a leading dot.

The loop uses i <= len(subdomainTokens) (Line 134), but when i == len(subdomainTokens), subdomainTokens[i:] is an empty slice. strings.Join([], ".") + "." + baseDomain produces ".example.com", which passes the newhost != "" check on Line 136 and results in a DNS query like <random>..example.com (double dot — invalid).

The existing IsWildcard (Line 33) correctly uses i < len(subdomainTokens). This should match.

🐛 Proposed fix
-		for i := 1; i <= len(subdomainTokens); i++ {
+		for i := 1; i < len(subdomainTokens); i++ {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var hosts []string
hosts = append(hosts, baseDomain)
if len(subdomainTokens) > 0 && subdomainTokens[0] != "" {
for i := 1; i <= len(subdomainTokens); i++ {
newhost := strings.Join(subdomainTokens[i:], ".") + "." + baseDomain
if newhost != "" {
hosts = append(hosts, newhost)
}
}
}
var hosts []string
hosts = append(hosts, baseDomain)
if len(subdomainTokens) > 0 && subdomainTokens[0] != "" {
for i := 1; i < len(subdomainTokens); i++ {
newhost := strings.Join(subdomainTokens[i:], ".") + "." + baseDomain
if newhost != "" {
hosts = append(hosts, newhost)
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/wildcard.go` around lines 130 - 140, The loop building hosts
in wildcard.go has an off-by-one: change the loop condition in the block that
iterates subdomainTokens (currently using i <= len(subdomainTokens)) to stop
before the slice becomes empty (use i < len(subdomainTokens>) so that
strings.Join(subdomainTokens[i:], ".") never returns an empty string); update
the loop that constructs newhost (the one referencing subdomainTokens,
baseDomain, and hosts) to match the logic used by IsWildcard so it doesn't
append a host like ".example.com".

…action

Addresses Neo Security Audit feedback about GetBaseDomain() function.
The previous naive approach using last 2 domain parts failed for multi-level TLDs like .co.uk.
Now uses github.com/weppos/publicsuffix-go for correct eTLD+1 extraction.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
internal/runner/wildcard.go (1)

148-151: ⚠️ Potential issue | 🟠 Major

Fix off-by-one in host generation (leading-dot host).

i <= len(subdomainTokens) allows an empty slice, producing ".<baseDomain>" and an invalid query. Match the IsWildcard loop and stop before the slice becomes empty.

🐛 Proposed fix
-		for i := 1; i <= len(subdomainTokens); i++ {
+		for i := 1; i < len(subdomainTokens); i++ {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/wildcard.go` around lines 148 - 151, The host-generation loop
in wildcard.go uses "for i := 1; i <= len(subdomainTokens); i++" which allows
subdomainTokens[i:] to be empty and produces a leading-dot host like
".<baseDomain>"; change the loop to stop before the slice becomes empty (e.g.,
"for i := 1; i < len(subdomainTokens); i++") so newhost :=
strings.Join(subdomainTokens[i:], ".") + "." + baseDomain never receives an
empty prefix; mirror the same boundary logic used in the IsWildcard loop to
prevent generating invalid queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/runner/wildcard.go`:
- Around line 84-88: The current logic calls r.dnsx.QueryOne(randomSubdomain)
and caches false via r.autoWildcardDomains.Set(domain, false) whenever err !=
nil || in == nil, which can store transient DNS failures as permanent
non-wildcard; change it to only cache false when the query succeeded but
returned nil (err == nil && in == nil). If err != nil (transient
timeout/servfail), do not write to r.autoWildcardDomains and simply return false
(or propagate the error if preferred), ensuring
r.autoWildcardDomains.Set(domain, false) is only invoked on a successful
negative lookup.

---

Duplicate comments:
In `@internal/runner/wildcard.go`:
- Around line 148-151: The host-generation loop in wildcard.go uses "for i := 1;
i <= len(subdomainTokens); i++" which allows subdomainTokens[i:] to be empty and
produces a leading-dot host like ".<baseDomain>"; change the loop to stop before
the slice becomes empty (e.g., "for i := 1; i < len(subdomainTokens); i++") so
newhost := strings.Join(subdomainTokens[i:], ".") + "." + baseDomain never
receives an empty prefix; mirror the same boundary logic used in the IsWildcard
loop to prevent generating invalid queries.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ef3fa1 and bfe7a9a.

📒 Files selected for processing (1)
  • internal/runner/wildcard.go

Comment on lines +84 to +88
in, err := r.dnsx.QueryOne(randomSubdomain)
if err != nil || in == nil {
// Random subdomain doesn't resolve, not a wildcard domain
_ = r.autoWildcardDomains.Set(domain, false)
return false
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid caching false on transient DNS errors.

If QueryOne fails (timeouts/servfail), caching false disables wildcard detection for the entire domain and can cause persistent false negatives. Consider only caching on successful negative lookups and skip caching on err != nil.

🔧 Proposed adjustment
 	in, err := r.dnsx.QueryOne(randomSubdomain)
-	if err != nil || in == nil {
-		// Random subdomain doesn't resolve, not a wildcard domain
-		_ = r.autoWildcardDomains.Set(domain, false)
-		return false
-	}
+	if err != nil {
+		// Transient failure: don't cache
+		return false
+	}
+	if in == nil {
+		// Random subdomain doesn't resolve, not a wildcard domain
+		_ = r.autoWildcardDomains.Set(domain, false)
+		return false
+	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/wildcard.go` around lines 84 - 88, The current logic calls
r.dnsx.QueryOne(randomSubdomain) and caches false via
r.autoWildcardDomains.Set(domain, false) whenever err != nil || in == nil, which
can store transient DNS failures as permanent non-wildcard; change it to only
cache false when the query succeeded but returned nil (err == nil && in == nil).
If err != nil (transient timeout/servfail), do not write to
r.autoWildcardDomains and simply return false (or propagate the error if
preferred), ensuring r.autoWildcardDomains.Set(domain, false) is only invoked on
a successful negative lookup.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support auto wildcard detection similar to PureDNS

1 participant