feat: implement honeypot detection mechanism#7056
Open
FuZoe wants to merge 2 commits intoprojectdiscovery:devfrom
Open
feat: implement honeypot detection mechanism#7056FuZoe wants to merge 2 commits intoprojectdiscovery:devfrom
FuZoe wants to merge 2 commits intoprojectdiscovery:devfrom
Conversation
Add honeypot detection to identify and mitigate hosts that return positive matches for an abnormally high number of templates. Features: - Add --honeypot-threshold (-hpt) flag to set detection threshold (percentage of templates matched by host to flag as honeypot) - Add --honeypot-suppress (-hps) flag to automatically suppress results from flagged honeypot hosts - Implement match density tracking per host with unique template counting - Add terminal warning: [HONEYPOT?] host matched X% of templates - Implement honeypot signature detection for common honeypots: Cowrie, Dionaea, Glastopf, Conpot, Elastichoney Usage: nuclei -hpt 30 -hps -t templates/ -l targets.txt Changes: - pkg/types/types.go: Add HoneypotThreshold and HoneypotSuppress fields to Options struct - cmd/nuclei/main.go: Add CLI flags in optimization group - pkg/protocols/common/honeypotcache/: New package for honeypot detection cache with tests - pkg/protocols/protocols.go: Add HoneypotCache to ExecutorOptions - internal/runner/runner.go: Initialize and close honeypot cache - pkg/tmplexec/exec.go: Integrate honeypot check in result writing Refs projectdiscovery#6403
Neo - PR Security ReviewNo security issues found Highlights
Comment |
Contributor
WalkthroughAdds honeypot detection and suppression: new CLI flags, a concurrent per-host honeypot cache with signature and threshold-based detection, integration into runner/executor to propagate the cache and optionally suppress results, plus tests for detection and normalization. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI/Main
participant Runner as Runner
participant Cache as HoneypotCache
participant Executor as Executor
participant Template as TemplateExec
CLI->>Runner: RunEnumeration(honeypotThreshold, honeypotSuppress)
alt threshold > 0
Runner->>Cache: New(threshold, suppress, maxHosts)
Runner->>Runner: store honeypotCache
end
Runner->>Runner: Load templates
alt Cache exists
Runner->>Cache: SetTotalTemplates(count)
end
Runner->>Executor: set HoneypotCache(cache)
loop for each template execution
Template->>Cache: Check(ctx)
alt Cache indicates honeypot
Cache-->>Template: Suppress result
Template->>Template: Skip WriteResult
else not honeypot
Template->>Template: Execute matchers
alt Match found
Template->>Cache: MarkMatch(ctx, templateID)
Cache->>Cache: update match count, evaluate %
alt threshold exceeded
Cache-->>Template: mark host honeypot
end
end
Template->>Executor: WriteResult
end
end
Runner->>Cache: Close()
Cache->>Cache: Log stats & purge
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/protocols/common/honeypotcache/honeypotcache.go (1)
103-115: Consider handling potential panic from cache iteration during concurrent access.The
GetALLcall inClose()may interact with concurrentSet()operations. While gcache is generally thread-safe, ensure this is intentional shutdown behavior where no more matches are being recorded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/common/honeypotcache/honeypotcache.go` around lines 103 - 115, Close may panic if matchedHosts.GetALL iterates while Set is called concurrently; before iterating, set a shutdown flag (e.g., add an atomic/boolean field on Cache like closing and set c.closing.Store(true) in Cache.Close) so Cache.Set() can return early when closing, then perform the GetALL iteration; additionally wrap the iteration in a defer-recover to catch/log any unexpected panic during GetALL to avoid crashing; update Cache.Set (or equivalent mutators) to check the closing flag and skip updates once closing is true, and keep existing calls to matchedHosts.Purge() and calculatePercentage/Threshold logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/nuclei/main.go`:
- Around line 438-439: The short alias "-hps" for HoneypotSuppress conflicts
with the existing short alias used by --http-stats; update the flag registration
for options.HoneypotSuppress (the flagSet.BoolVarP call) to use a unique
single-character short name (e.g., change "hps" to an unused single-letter like
"y" or "z"), and ensure no other flag in the same flagSet uses that letter;
adjust only the BoolVarP invocation for HoneypotSuppress and verify there are no
remaining duplicate short aliases (leave --http-stats unchanged).
In `@pkg/protocols/common/honeypotcache/honeypotcache.go`:
- Around line 180-211: MarkMatch has a race on initializing per-host cache via
the GetIFPresent/Set pattern on matchedHosts: concurrent goroutines can each
create separate hostMatches and overwrite each other. Replace the manual
GetIFPresent/Set logic in MarkMatch by using the gcache LoaderFunc-based atomic
initialization (configure matchedHosts in New() with LoaderFunc returning
&hostMatches{matches: make(map[string]bool)}), then use
matchedHosts.Get(cacheKey) to retrieve the *hostMatches so creation is
singleflight/atomic; keep subsequent cache.mu locking and updates (matchCount,
matches map, flaggedWarning) unchanged.
In `@pkg/protocols/protocols.go`:
- Around line 102-103: The new HoneypotCache field of ExecutorOptions isn't
being propagated when creating copies or when reapplying engine options; update
the Copy() method and ApplyNewEngineOptions() to copy/assign the HoneypotCache
the same way HostErrorsCache is handled so the cache is preserved across copies
and option reapplication (ensure you mirror the nil-safety/assignment logic used
for HostErrorsCache when setting HoneypotCache on the returned ExecutorOptions
and when applying new engine options).
---
Nitpick comments:
In `@pkg/protocols/common/honeypotcache/honeypotcache.go`:
- Around line 103-115: Close may panic if matchedHosts.GetALL iterates while Set
is called concurrently; before iterating, set a shutdown flag (e.g., add an
atomic/boolean field on Cache like closing and set c.closing.Store(true) in
Cache.Close) so Cache.Set() can return early when closing, then perform the
GetALL iteration; additionally wrap the iteration in a defer-recover to
catch/log any unexpected panic during GetALL to avoid crashing; update Cache.Set
(or equivalent mutators) to check the closing flag and skip updates once closing
is true, and keep existing calls to matchedHosts.Purge() and
calculatePercentage/Threshold logic unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/nuclei/main.gointernal/runner/runner.gopkg/protocols/common/honeypotcache/honeypotcache.gopkg/protocols/common/honeypotcache/honeypotcache_test.gopkg/protocols/protocols.gopkg/tmplexec/exec.gopkg/types/types.go
cmd/nuclei/main.go
Outdated
Comment on lines
438
to
439
| flagSet.IntVarP(&options.HoneypotThreshold, "honeypot-threshold", "hpt", 0, "honeypot detection threshold (percentage of templates matched by host to flag as honeypot)"), | ||
| flagSet.BoolVarP(&options.HoneypotSuppress, "honeypot-suppress", "hps", false, "suppress results from hosts flagged as honeypots"), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -B2 -A2 'hps|http-stats' cmd/nuclei/main.go | head -50Repository: projectdiscovery/nuclei
Length of output: 868
🏁 Script executed:
rg -c -n 'hps' cmd/nuclei/main.goRepository: projectdiscovery/nuclei
Length of output: 68
🏁 Script executed:
rg -n '"hps"' cmd/nuclei/main.goRepository: projectdiscovery/nuclei
Length of output: 323
Alias conflict: -hps is already used by --http-stats on line 485.
The -hps short flag alias on line 439 duplicates the one used by --http-stats on line 485. Go's pflag library does not permit duplicate short flag aliases within the same flagSet, causing the CLI to fail at startup.
🐛 Proposed fix: Use a different alias
- flagSet.BoolVarP(&options.HoneypotSuppress, "honeypot-suppress", "hps", false, "suppress results from hosts flagged as honeypots"),
+ flagSet.BoolVarP(&options.HoneypotSuppress, "honeypot-suppress", "hpsu", false, "suppress results from hosts flagged as honeypots"),📝 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
| flagSet.IntVarP(&options.HoneypotThreshold, "honeypot-threshold", "hpt", 0, "honeypot detection threshold (percentage of templates matched by host to flag as honeypot)"), | |
| flagSet.BoolVarP(&options.HoneypotSuppress, "honeypot-suppress", "hps", false, "suppress results from hosts flagged as honeypots"), | |
| flagSet.IntVarP(&options.HoneypotThreshold, "honeypot-threshold", "hpt", 0, "honeypot detection threshold (percentage of templates matched by host to flag as honeypot)"), | |
| flagSet.BoolVarP(&options.HoneypotSuppress, "honeypot-suppress", "hpsu", false, "suppress results from hosts flagged as honeypots"), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/nuclei/main.go` around lines 438 - 439, The short alias "-hps" for
HoneypotSuppress conflicts with the existing short alias used by --http-stats;
update the flag registration for options.HoneypotSuppress (the flagSet.BoolVarP
call) to use a unique single-character short name (e.g., change "hps" to an
unused single-letter like "y" or "z"), and ensure no other flag in the same
flagSet uses that letter; adjust only the BoolVarP invocation for
HoneypotSuppress and verify there are no remaining duplicate short aliases
(leave --http-stats unchanged).
Author
Test ResultsAll honeypotcache tests pass: 
|
- Change -hps to -hpsu to avoid alias conflict with --http-stats - Use LoaderFunc for atomic cache initialization (fix race condition) - Propagate HoneypotCache in Copy() and ApplyNewEngineOptions()
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/protocols/common/honeypotcache/honeypotcache.go (1)
125-152:⚠️ Potential issue | 🟡 MinorNormalize cache keys consistently to avoid host key fragmentation.
NormalizeCacheValuepreserves host casing, so the same host can be split across keys (for example, mixed-case inputs), which undercounts matches and can delay honeypot flagging.💡 Suggested fix
func (c *Cache) NormalizeCacheValue(value string) string { - var normalizedValue = value + var normalizedValue = strings.TrimSpace(value) u, err := url.ParseRequestURI(value) if err != nil || u.Host == "" { if strings.Contains(value, ":") { return normalizedValue } u, err2 := url.ParseRequestURI("https://" + value) if err2 != nil { return normalizedValue } - normalizedValue = u.Host + host := strings.ToLower(u.Hostname()) + if port := u.Port(); port != "" { + normalizedValue = net.JoinHostPort(host, port) + } else { + normalizedValue = host + } } else { - port := u.Port() + host := strings.ToLower(u.Hostname()) + port := u.Port() if port == "" { switch u.Scheme { case "https": - normalizedValue = net.JoinHostPort(u.Host, "443") + normalizedValue = net.JoinHostPort(host, "443") case "http": - normalizedValue = net.JoinHostPort(u.Host, "80") + normalizedValue = net.JoinHostPort(host, "80") + default: + normalizedValue = host } } else { - normalizedValue = u.Host + normalizedValue = net.JoinHostPort(host, port) } } return normalizedValue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/common/honeypotcache/honeypotcache.go` around lines 125 - 152, NormalizeCacheValue currently preserves host casing and can fragment cache keys; fix by lowercasing any host-derived parts before assigning normalizedValue. Specifically, when you parse a URL (using url.ParseRequestURI) and read u.Host or u.Port(), use u.Hostname() and do strings.ToLower(u.Hostname()) and then rebuild the host+port with net.JoinHostPort(lowercaseHostname, port) (or set normalizedValue to strings.ToLower(u.Host) when no port logic applies) so all host-based normalizedValue assignments are canonicalized to lowercase.
🧹 Nitpick comments (1)
pkg/protocols/common/honeypotcache/honeypotcache.go (1)
98-101:SetVerboseis currently a no-op.
SetVerbosesetsc.verbose, but log emission ignores it. Either gate honeypot logs withc.verboseor remove this API to avoid misleading behavior.Also applies to: 117-117, 179-180, 213-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/common/honeypotcache/honeypotcache.go` around lines 98 - 101, SetVerbose currently stores the flag but logging code ignores it; either honor the flag or remove the API. Fix by locating the Cache struct's log emission sites (any calls that use the cache logger inside Cache methods) and wrap those log calls with if c.verbose { ... } (or check c.verbose before calling c.logger.*), or alternatively remove the SetVerbose method and any callers so there’s no misleading API. Ensure you update all related log sites referenced in the review (the other Cache methods that emit honeypot logs) and keep the Cache.verbose field and SetVerbose in sync with that change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/protocols/common/honeypotcache/honeypotcache.go`:
- Around line 125-152: NormalizeCacheValue currently preserves host casing and
can fragment cache keys; fix by lowercasing any host-derived parts before
assigning normalizedValue. Specifically, when you parse a URL (using
url.ParseRequestURI) and read u.Host or u.Port(), use u.Hostname() and do
strings.ToLower(u.Hostname()) and then rebuild the host+port with
net.JoinHostPort(lowercaseHostname, port) (or set normalizedValue to
strings.ToLower(u.Host) when no port logic applies) so all host-based
normalizedValue assignments are canonicalized to lowercase.
---
Nitpick comments:
In `@pkg/protocols/common/honeypotcache/honeypotcache.go`:
- Around line 98-101: SetVerbose currently stores the flag but logging code
ignores it; either honor the flag or remove the API. Fix by locating the Cache
struct's log emission sites (any calls that use the cache logger inside Cache
methods) and wrap those log calls with if c.verbose { ... } (or check c.verbose
before calling c.logger.*), or alternatively remove the SetVerbose method and
any callers so there’s no misleading API. Ensure you update all related log
sites referenced in the review (the other Cache methods that emit honeypot logs)
and keep the Cache.verbose field and SetVerbose in sync with that change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/nuclei/main.gopkg/protocols/common/honeypotcache/honeypotcache.gopkg/protocols/protocols.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/protocols/protocols.go
Author
|
Hi maintainers, I have only one active PR and have addressed all CodeRabbit feedback. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add honeypot detection to identify and mitigate hosts that return positive matches for an abnormally high number of templates.
Features:
Usage:
nuclei -hpt 30 -hps -t templates/ -l targets.txt
Changes:
Refs #6403
Proposed changes
This PR implements honeypot detection as requested in #6403.
The implementation includes:
Proof
go test ./pkg/protocols/... ./pkg/tmplexec/... ./internal/runner/...)go build ./...)Checklist
Summary by CodeRabbit
New Features
Tests