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.

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

Exclude Raw Request Payloads #3710

Merged
merged 14 commits into from
Jul 4, 2023

Conversation

kchason
Copy link
Contributor

@kchason kchason commented May 19, 2023

Proposed changes

Addresses #3709

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@kchason kchason changed the title Exclude raw request payloads Exclude Raw Request Payloads May 19, 2023
@ehsandeep ehsandeep linked an issue May 22, 2023 that may be closed by this pull request
Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

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

lgtm !

$ ./nuclei -u oast.site -id "robots-txt-endpoint" -je results.json -erp && cat results.json| jq .   3 ↵

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v2.9.4-dev

		projectdiscovery.io

[WRN] Found 24 templates loaded with deprecated paths, update before v2.9.5 for continued support.
[WRN] Found 1 templates loaded with deprecated protocol syntax, update before v2.9.5 for continued support.
[INF] Current nuclei version: v2.9.4-dev (outdated)
[INF] Current nuclei-templates version: v9.5.0 (latest)
[INF] New templates added in latest release: 62
[INF] Templates loaded for current scan: 1
[INF] Targets loaded for current scan: 1
[INF] Running httpx on input host
[INF] Found 1 URL from httpx
[robots-txt-endpoint] [http] [info] https://oast.site/robots.txt
[
  {
    "template": "http/miscellaneous/robots-txt-endpoint.yaml",
    "template-url": "https://github.com/projectdiscovery/nuclei-templates/blob/main/http/miscellaneous/robots-txt-endpoint.yaml",
    "template-id": "robots-txt-endpoint",
    "template-path": "/Users/tarun/nuclei-templates/http/miscellaneous/robots-txt-endpoint.yaml",
    "info": {
      "name": "robots.txt endpoint prober",
      "author": [
        "caspergn",
        "pdteam"
      ],
      "tags": null,
      "reference": null,
      "severity": "info",
      "metadata": {
        "max-request": 2
      }
    },
    "type": "http",
    "host": "https://oast.site",
    "matched-at": "https://oast.site/robots.txt",
    "ip": "178.128.16.97",
    "timestamp": "2023-05-23T00:11:10.354518+05:30",
    "curl-command": "curl -X 'GET' -d '' -H 'Accept: */*' -H 'Accept-Language: en' -H 'User-Agent: Mozilla/5.0 (Windows NT 10.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.93 Safari/537.36' 'https://oast.site/robots.txt'",
    "matcher-status": true,
    "matched-line": null
  }
]

@tarunKoyalwar
Copy link
Member

@ehsandeep , i think we can improve flag description to explicitly tell that -erp is only used/affects reporting modules/exporters and not output-jsonl

@kchason
Copy link
Contributor Author

kchason commented May 24, 2023

Is there a change requested on this to emphasize that it's the reports only and not the console formatted JSON (which I don't believe it includes anyway), or is that under review?

-erp, -exclude-raw-payload whether to exclude the raw request and response payloads from the JSON, JSONL, and markdown outputs

Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

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

@kchason @tarunKoyalwar nuclei already has -irr option i.e

   -irr, -include-rr  include request/response pairs in the JSONL output (for findings only)

so instead of adding a new option to exclude, we can default to exclude raw requests/responses and only include when -irr option is used (similar to -j option) for export feature as well.

@kchason
Copy link
Contributor Author

kchason commented Jun 6, 2023

Should be all set now. I changed the internal variable name to better match what it's doing as well, it doesn't change anyone's integrations since the flag is the same

@tarunKoyalwar tarunKoyalwar requested review from Mzack9999 and removed request for tarunKoyalwar June 13, 2023 12:57
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

I think the default behavior should be to include them and skin them optionally. What do you think @ehsandeep ?

@Mzack9999 Mzack9999 requested a review from ehsandeep June 15, 2023 08:37
@kchason
Copy link
Contributor Author

kchason commented Jun 15, 2023

@Mzack9999 that wouldn't match the existing functionality of the -include-rr, -irr flags then though, right?

@ehsandeep ehsandeep deleted the branch projectdiscovery:dev June 27, 2023 16:30
@ehsandeep ehsandeep closed this Jun 27, 2023
@kchason
Copy link
Contributor Author

kchason commented Jun 27, 2023

Is this not accepted as a concept? Or are there changes requested on this?

@ehsandeep ehsandeep reopened this Jun 27, 2023
@ehsandeep
Copy link
Member

@kchason was closed by mistake.

Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

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

@kchason as mzack suggested, it would be nice (also to align with our default behavior across project) to include all the information as default and optionally exclude things, also to keep the uniform behavior across all the output events which is not the case for now, for example -irr is enabled as default for reporting and markdown but not for jsonl.

Can you adapt the PR as per #3891? let me know if you have any questions.

@ehsandeep ehsandeep added Status: Revision Needed Submitter of PR needs to revise the PR related to the issue. Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity. labels Jul 3, 2023
@kchason
Copy link
Contributor Author

kchason commented Jul 3, 2023

The PR has been updated with:

  • Changing the default value of -irr, -include-rr to true
  • Added [DEPRECATED] to the -irr, -include-rr flag
  • Added a new -or, -omit-raw flag

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

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

lgtm !

$  ./nuclei -u oast.site -id "robots-txt-endpoint" -or -je results.json  && cat results.json| jq . 

                     __     _
   ____  __  _______/ /__  (_)
  / __ \/ / / / ___/ / _ \/ /
 / / / / /_/ / /__/ /  __/ /
/_/ /_/\__,_/\___/_/\___/_/   v2.9.8

		projectdiscovery.io

[WRN] Found 3 templates loaded with deprecated protocol syntax, update before v3 for continued support.
[INF] Current nuclei version: v2.9.8 (latest)
[INF] Current nuclei-templates version: v9.5.4 (latest)
[INF] New templates added in latest release: 51
[INF] Templates loaded for current scan: 1
[INF] Targets loaded for current scan: 1
[INF] Running httpx on input host
[INF] Found 1 URL from httpx
[robots-txt-endpoint] [http] [info] https://oast.site/robots.txt
[
  {
    "template": "http/miscellaneous/robots-txt-endpoint.yaml",
    "template-url": "https://github.com/projectdiscovery/nuclei-templates/blob/main/http/miscellaneous/robots-txt-endpoint.yaml",
    "template-id": "robots-txt-endpoint",
    "template-path": "/Users/tarun/nuclei-templates/http/miscellaneous/robots-txt-endpoint.yaml",
    "info": {
      "name": "robots.txt endpoint prober",
      "author": [
        "caspergn",
        "pdteam"
      ],
      "tags": [
        "misc",
        "generic"
      ],
      "reference": null,
      "severity": "info",
      "metadata": {
        "max-request": 2
      }
    },
    "type": "http",
    "host": "https://oast.site",
    "matched-at": "https://oast.site/robots.txt",
    "ip": "178.128.16.97",
    "timestamp": "2023-07-05T01:20:09.794878+05:30",
    "curl-command": "curl -X 'GET' -d '' -H 'Accept: */*' -H 'Accept-Language: en' -H 'User-Agent: Mozilla/5.0 (Windows NT 10.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.114 Safari/537.36' 'https://oast.site/robots.txt'",
    "matcher-status": true,
    "matched-line": null
  }
]

@ehsandeep ehsandeep removed the Status: Revision Needed Submitter of PR needs to revise the PR related to the issue. label Jul 4, 2023
@ehsandeep ehsandeep merged commit b3ccb9a into projectdiscovery:dev Jul 4, 2023
11 checks passed
@ehsandeep
Copy link
Member

Thank you @kchason for adding this.

@kchason kchason deleted the exclude-raw-request-payloads branch July 5, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity.
Projects
None yet
4 participants