Add --auto-wildcard option with per-domain wildcard detection#940
Add --auto-wildcard option with per-domain wildcard detection#940Excellencedev wants to merge 3 commits intoprojectdiscovery:devfrom
--auto-wildcard option with per-domain wildcard detection#940Conversation
…ction-feature Add `--auto-wildcard` option with per-domain wildcard detection
WalkthroughThis PR introduces an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
cc @dogancanbakir Please review |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/runner/runner.go (1)
499-637:⚠️ Potential issue | 🟡 Minor
filterWildcardssilently drops non-JSON output formatting (e.g.-resp,-ro).
lookupAndOutput(line 648–667) only emits raw hostnames in non-JSON mode. When--auto-wildcardis active, users who also pass-a -respor-resp-onlywill get bare hostnames instead of formatted DNS responses. The existing-wdflag documents this limitation ("only json output is supported"), but--auto-wildcardhas no equivalent warning or enforcement.Consider either:
- Adding a validation error or warning when
--auto-wildcardis combined with response-formatting flags, or- Documenting the JSON-only limitation for
--auto-wildcardin the help text and README.
🧹 Nitpick comments (4)
internal/runner/wildcard_test.go (1)
5-25: Consider usingt.Runfor subtests and adding edge cases.The test table is solid but could benefit from two improvements:
- Using
t.Runwith a descriptive name per case gives clearer failure output and allows running individual cases.- Consider adding edge cases: empty string
"", an IPv6 address like"::1", and a two-level ccTLD without a known second-level (e.g.,"example.uk"→"example.uk").♻️ Suggested improvement
for _, tt := range tests { - if got := wildcardBaseDomain(tt.input); got != tt.expected { - t.Errorf("wildcardBaseDomain(%q) = %q, want %q", tt.input, got, tt.expected) - } + t.Run(tt.input, func(t *testing.T) { + t.Parallel() + if got := wildcardBaseDomain(tt.input); got != tt.expected { + t.Errorf("wildcardBaseDomain(%q) = %q, want %q", tt.input, got, tt.expected) + } + }) }internal/runner/runner.go (2)
588-607: Dead code / unreachable branches inunfilteredHostsprocessing.
unfilteredHostsis only populated whenr.options.AutoWildcardis true and the host is an IP or has no dot (lines 520–522). In the output loop:
- Line 593:
r.options.AutoWildcardis always true for these hosts.- Line 594:
wildcardBaseDomain(host)will always return""for IPs and single-label hosts.- Line 596: The IP/no-dot check is always true (that's how they entered
unfilteredHosts).- Lines 598–603 and 606 are unreachable.
This doesn't cause incorrect behavior, but the redundant re-derivation makes the logic harder to follow. Consider simplifying:
♻️ Simplified unfilteredHosts output
- for host := range unfilteredHosts { - if _, ok := seen[host]; ok { - continue - } - seen[host] = struct{}{} - if r.options.AutoWildcard { - wildcardDomain := wildcardBaseDomain(host) - if wildcardDomain == "" { - if net.ParseIP(host) != nil || !strings.Contains(host, ".") { - _ = r.lookupAndOutput(host) - } else { - ambiguousHosts[host] = struct{}{} - } - } else { - ambiguousHosts[host] = struct{}{} - } - continue - } - _ = r.lookupAndOutput(host) - } + for host := range unfilteredHosts { + if _, ok := seen[host]; ok { + continue + } + seen[host] = struct{}{} + _ = r.lookupAndOutput(host) + }
551-580: Edge case: whentotalIPs == 0the wildcard worker channel is closed without starting workers, which is correct — butwgwildcardworkeris never waited on.When
totalIPs > 0, workers are started (line 559–561) and waited on (line 577). WhentotalIPs == 0, the channel is closed at line 579 butr.wgwildcardworker.Wait()is never called. This is technically fine since no workers were added, but for symmetry and safety against future changes, consider callingr.wgwildcardworker.Wait()after the else branch as well.internal/runner/wildcard.go (1)
10-44: Consider usinggolang.org/x/net/publicsuffixfor more reliable domain parsing.The
commonSecondLevelDomains+len(last) == 2heuristic handles common cases well (.co.uk,.com.br,.co.in) but can misfire on edge cases:
- Two-letter gTLDs where the second label matches the map: e.g.
sub.example.co.de→"example.co.de"(treats.co.deas a hierarchical ccTLD, which is incorrect).- Three+ letter ccTLD compounds: e.g.
sub.example.co.com→"co.com"(loses theexamplelabel).The
golang.org/x/net/publicsuffixpackage provides an authoritative Public Suffix List and would eliminate these edge cases. TheEffectiveTLDPlusOne()function returns the registrable domain directly:import "golang.org/x/net/publicsuffix" func wildcardBaseDomain(host string) string { host = strings.TrimSpace(strings.TrimSuffix(host, ".")) if host == "" || iputil.IsIP(host) { return "" } domain, err := publicsuffix.EffectiveTLDPlusOne(strings.ToLower(host)) if err != nil { return "" } return domain }This is a recommended improvement but not blocking given the heuristic's scope targets common cases.
|
This is the one issue I would like to work on |
Motivation
-wdper domain.Description
AutoWildcardoption and CLI flag--auto-wildcard, plus validation to prevent incompatible combinations like--streamor-wdtogether with--auto-wildcard.wildcardBaseDomain(host)heuristic to compute a base domain (handles common 2nd-level TLD cases) and change the wildcard pipeline to operate per base domain.wildcardJobtype and job channel are added, andIsWildcardsignature updated to accept thewildcardDomainparameter so workers can run per-domain checks.lookupAndOutputflow to output filtered results.internal/runner/wildcard_test.gofor the base-domain heuristic and document the feature and example usage inREADME.mdwith--auto-wildcardusage.Testing
internal/runner/wildcard_test.gocoveringwildcardBaseDomain; test file included in the PR.gofmton modified files successfully.Test Logs :
$ go test ./internal/runner -run TestWildcardBaseDomain ok github.com/projectdiscovery/dnsx/internal/runner 1.763s/fixes #924
/claim #924
Summary by CodeRabbit
New Features
--auto-wildcardflag for automatic detection and filtering of wildcard subdomains per base domainDocumentation
--auto-wildcardflag functionality--auto-wildcardfor wildcard elimination