PortSwigger / BChecks

BChecks collection for Burp Suite Professional and Burp Suite Enterprise Edition
https://portswigger.net/burp/documentation/scanner/bchecks
GNU Lesser General Public License v3.0
619 stars 109 forks source link

Create InsecureContentSecurityPolicy.bcheck #129

Closed LabMC closed 12 months ago

LabMC commented 12 months ago

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.

LabMC commented 12 months ago

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:

Thanks, KG,

Hannah-PortSwigger commented 12 months ago

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.

LabMC commented 12 months ago

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.