Skip to content

Conversation

@DhiyaneshGeek
Copy link
Member

Template / PR Information

  • Fixed CVE-2020-XXX / Added CVE-2020-XXX / Updated CVE-2020-XXX
  • References:

Template Validation

I've validated this template locally?

  • YES
  • NO

Additional Details (leave it blank if not applicable)

Additional References:

@ritikchaddha ritikchaddha merged commit ae82d13 into main Dec 2, 2023
@ritikchaddha ritikchaddha deleted the fix-matcher-matrix branch December 2, 2023 14:18
- '"name":"Synapse"'
- '"server":'
- '"name":'
- '"version":'
Copy link
Contributor

Choose a reason for hiding this comment

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

If think this approach is quite costly. Did you find something that answered 200 to this exact path but without the keys and that made the extractor work?

I tried to run this against an endpoint which had the correct path but without the correct content and no false positives.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @davidegirardi , Is it possible share more details around this in discord to me ?

Looking forward to hear back from you

Thanks 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I get a 404 from that link.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found an invite link on your site and I'm not going to share my phone number via Discord just to contribute a template that was working just fine and was tested for days on thousands of hosts with zero false positives.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @davidegirardi , you can share the debug data of the missed detection here in the comment

thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not talking about missed detection. I am talking about performance. Why should we do multiple string matchings on the body to search for something that the extractor actually uses? It's a waste of CPU time at each probe.

If those strings are not in the body, the extractor will not do anything and there will be no detection of a Matrix homeserver, nor it's version. As expected.

Moreover, I really doubt there are services other than a Matrix server answering on that endpoint.

Copy link
Contributor

Choose a reason for hiding this comment

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

If, for some reason, we must have a matcher, this one should be more efficient:

    matchers:

      - type: dsl
        dsl:
          - status_code_1 == 200

I still think it's a waste since it's implicit.

Copy link
Member

Choose a reason for hiding this comment

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

@davidegirardi while writing templates we not only need to check about true-positive matches, but also need to consider where it can be possibly have unexpected matches, unexpected matches are common when you run any templates at large scale where random honeypot server trying to mimic incoming request. that's why it's important to have unique matches to each template specfic to that detection only.

to give you example, above status based check will match against one of our test server http://honey.scanme.sh

echo http://honey.scanme.sh | nuclei -t http/technologies/matrix-homeserver-detect.yaml

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v3.1.0

		projectdiscovery.io

[INF] Current nuclei version: v3.1.0 (latest)
[INF] Current nuclei-templates version: v9.7.1 (latest)
[WRN] Scan results upload to cloud is disabled.
[INF] New templates added in latest release: 0
[INF] Templates loaded for current scan: 1
[WRN] Executing 1 unsigned templates. Use with caution.
[INF] Targets loaded for current scan: 1
[matrix-homeserver-detect] [http] [info] http://honey.scanme.sh/_matrix/federation/v1/version

While the extractor you added is unique and will not just match any server, adding matchers is still needed for two primary reasons:

  1. Template Readability - Matchers clarify what specifically is being checked to detect certain technology or vulnerabilities in the template.

  2. Performance - Using only extractors will lead to higher CPU usage. Matchers, particularly, word matcher improves performance as it only looks for string matches, hence limiting the scope for extractors. Without matchers, Nuclei would process extractors on all responses, not just on valid detection.

Even from feature point of view, extractors in nuclei are added to be worked on the top of matcher.

Copy link
Contributor

@davidegirardi davidegirardi Dec 4, 2023

Choose a reason for hiding this comment

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

A bit of feedback then: part of what you wrote above should have been the content of a review for my original PR.

The two comments about performance would have been enough, with the readability as the optional cherry on top.

Luckily enough my other PR for Element Web detection already has matchers using contains() since we are dealing with larger and more varied responses there.

EDIT: having the discussion here could probably help someone in the future since they have a possibility to actually find what we talked about.

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

Labels

Done Ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants