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

Create InsecureContentSecurityPolicy.bcheck #129

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Create InsecureContentSecurityPolicy.bcheck #129

merged 3 commits into from
Oct 9, 2023

Conversation

LabMC
Copy link
Contributor

@LabMC LabMC commented Oct 8, 2023

  • The purpose of this BCheck aims for permitting users to discover if scanned HTTP responses either:
  • Info: Are missing a Content-Security-Policy.
  • Low: Are using known-insecure values in a Content-Security-Policy (unsafe-inline, unsafe-eval, wildcards).
  • Low: Are using known-deprecated values in a Content-Security-Policy.

Additional testing has been performed to reduce the amount of False Positives from Missing-CSP checks performed in static files (such as images, fonts, plaintext).

Please inform me if any additional information or cutdown appears necessary. Thank you.

- The purpose of this BCheck aims for permitting users to discover if scanned HTTP responses either:
 - Info:  Are missing a Content-Security-Policy.
 - Low:  Are using known-insecure values in a Content-Security-Policy (unsafe-inline, unsafe-eval, wildcards).
 - Low:  Are using known-deprecated values in a Content-Security-Policy.

Additional testing has been performed to reduce the amount of False Positives from Missing-CSP checks performed in static files (such as images, fonts, plaintext).

Inform me if any additional information or cutdown appears necessary.
Copy link
Collaborator

@PortSwiggerWiener PortSwiggerWiener left a comment

Choose a reason for hiding this comment

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

Thanks for the BCheck submission. It looks well thought out and structured.

I've tried to validate the script and am currently getting some errors due to insecure_value not being defined within the scope of the define block:

  • Unexpected token insecure_value on line 11 position 41
  • Unexpected token insecure_value on line 16 position 28
  • Unexpected token insecure_value on line 19 position 47
  • Unexpected token insecure_value on line 27 position 86
  • Unexpected token insecure_value on line 28 position 155
  • Unexpected token insecure_value on line 29 position 65

Can you please see if you get the same result either using the BCheck testing tool (currently on the Early Adopter channel) or by using the BCheckChecker JAR within this repo? If you need help using either then just let us know!

@LabMC
Copy link
Contributor Author

LabMC commented Oct 8, 2023

Thanks for the BCheck submission. It looks well thought out and structured.

I've tried to validate the script and am currently getting some errors due to insecure_value not being defined within the scope of the define block:

  • Unexpected token insecure_value on line 11 position 41
  • Unexpected token insecure_value on line 16 position 28
  • Unexpected token insecure_value on line 19 position 47
  • Unexpected token insecure_value on line 27 position 86
  • Unexpected token insecure_value on line 28 position 155
  • Unexpected token insecure_value on line 29 position 65

Can you please see if you get the same result either using the BCheck testing tool (currently on the Early Adopter channel) or by using the BCheckChecker JAR within this repo? If you need help using either then just let us know!


Thank you for the reply,
I used Burp Professional's BChecks functionality to test this script. I originally had "run for each:" BEFORE "define:". But I think I assumed that using "run for each:" before "define" would cause the "define" variables to become re-called after each "run for each".

THAT BEING SAID, I confirmed prior to submitting this file that placing "define:" before "run for each" while using "insecure_value" still populated "cspVal" correctly when using Burp Professional. (I didn't test this with BCheck Jar).

confirmCSP

If you found these errors through BCheckChecker JAR, maybe this signifies behavior unique to BCheckChecker JAR that Burp Professional doesn't have?
In any case, it looks like simply switching "define:" and "run for each:" around would solve this issue.

Before I do that, can you confirm for me:

  • If using "define:" after "run for each:" means "define:" is being re-called per for-loop?

Thanks,
KG,

@Hannah-PortSwigger
Copy link
Contributor

Hi.

Whilst your BCheck as it is does work with the Scanner, we've made the validation of BChecks stricter in our current version of Early Adopter which is what our "BCheckChecker" JAR lines up with. This is to encourage better practices and reduce complexity, and will be coming to the Stable channel soon. Once this change is on our Stable channel, you will not be able to load your BCheck as it will not pass the validation.

I've double-checked, and the order that the "define" and "run for each" block are in does not make a difference. By swapping these around in your BCheck, you will not have an impact on performance, but you will pass the unreferenced value validation check.

If you have any questions, then please let us know.

Switched Around Define & Run-For-Each
@LabMC
Copy link
Contributor Author

LabMC commented Oct 9, 2023

Hi.

Whilst your BCheck as it is does work with the Scanner, we've made the validation of BChecks stricter in our current version of Early Adopter which is what our "BCheckChecker" JAR lines up with. This is to encourage better practices and reduce complexity, and will be coming to the Stable channel soon. Once this change is on our Stable channel, you will not be able to load your BCheck as it will not pass the validation.

I've double-checked, and the order that the "define" and "run for each" block are in does not make a difference. By swapping these around in your BCheck, you will not have an impact on performance, but you will pass the unreferenced value validation check.

If you have any questions, then please let us know.

Thanks for the clarification,
I have the "define" & "run for each" blocks switched around now. 👍
Let me know if you need anything else regarding this bcheck file.

Copy link
Contributor

@Hannah-PortSwigger Hannah-PortSwigger left a comment

Choose a reason for hiding this comment

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

Thanks for making that change! This looks good 👍

Copy link
Collaborator

@PortSwiggerWiener PortSwiggerWiener left a comment

Choose a reason for hiding this comment

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

Thanks for your patience. Much appreciated 👍

@PortSwiggerWiener PortSwiggerWiener merged commit 1753cd8 into PortSwigger:main Oct 9, 2023
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