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

Added Text4Shell detection rule #178

Closed
wants to merge 4 commits into from
Closed

Conversation

GiriRaj249
Copy link
Contributor

@GiriRaj249 GiriRaj249 commented Feb 9, 2024

BCheck Contributions

  • BCheck compiles and executes as expected
  • BCheck contains appropriate metadata (name, version, author, description and appropriate tags)
  • Only .bcheck files have been added or modified
  • BCheck is in the appropriate folder
  • PR contains single or limited number of BChecks (Multiple PRs are preferred)
  • BCheck attempts to minimize false positives

@PortSwiggerWiener
Copy link
Collaborator

Many thanks for your submission. It looks interesting and it's always good to see the Collaborator being used!

Main comment is that you probably need to write either an insertion point based check or a request based one as it's a bit of a mix at the moment. Both have pros and cons:

  1. Insertion point based (given insertion point) - these are the most fine-grained BChecks and will be executed for each insertion point that the scanner finds in the base request (which depends on the scan config). As on lines 17-18, you insert / append your payload in the insertion point.

  2. Request based (given request) - these are more coarse grained and will be run for each unique request (audit item) given to the scanner. With these, you'd configure your payload request as you're doing on lines 19-27.

Currently the BCheck appends the payload to the insertion point before issuing this request and then issues a separate request with the payload in the headers. This latter request will be identical to other requests when the base request has multiple insertion points.

I'd recommend thinking about which route you want to go down and simplifying the BCheck.

Of course, if I've misunderstood things then do please let me know!

@Hannah-PortSwigger
Copy link
Contributor

Hi there!

Have you had a chance to review the feedback given?

Unfortunately, without this feedback implemented, we are not able to merge this pull request into the main repository.

If there's anything that we can help with, then please let us know.

@Hannah-PortSwigger
Copy link
Contributor

Unfortunately, we will have to close this pull request. We're happy to reopen this after the feedback has been reviewed.

If there's anything that we can help with, then please let us know.

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

Successfully merging this pull request may close these issues.

3 participants