Hi Andres,
I answer into your mail using !!!> prefix
Dom,
This is my code review for the CSP tests and utils:
* All the tests passed without any modifications
* Code in utils.py looks very clean, great improvement
!!!> Thanks you very much ;)
* Code in test_utils.py did not respect the 80 column limit, applied
autopep8 to it to fix it
!!!> Sorry I will also use this tools
* This is kind of hard to read:
hrds[CSP_HEADER_FIREFOX] = CSP_DIRECTIVE_IMAGE + " '" + \
CSP_DIRECTIVE_VALUE_UNSAFE_INLINE + "'"
hrds[CSP_HEADER_W3C_REPORT_ONLY] = CSP_DIRECTIVE_DEFAULT + \
" 'self';" + CSP_DIRECTIVE_REPORT_URI + " http://example.com";
hrds[CSP_HEADER_W3C] = CSP_DIRECTIVE_SCRIPT + " 'self';" + \
CSP_DIRECTIVE_REPORT_URI + " /myrelativeuri"
For the next contribution, I think that it would be fine to have tests
that don't use the constants and instead look like:
hrds[CSP_HEADER_FIREFOX] = "img-src 'unsafe-inline'"
hrds[CSP_HEADER_W3C_REPORT_ONLY] = "default-src 'self';
report-uri http://example.com";
...
!!!> OK, excuse me is a professional bias due to my daily job ;)
* Question: "Do we need these extra lines in
test_unsafe_inline_enabled_no_case02?"
hrds[CSP_HEADER_W3C_REPORT_ONLY] = CSP_DIRECTIVE_DEFAULT + \
" 'self';" + CSP_DIRECTIVE_REPORT_URI + " http://example.com";
hrds[CSP_HEADER_W3C] = CSP_DIRECTIVE_SCRIPT + " 'self';" + \
CSP_DIRECTIVE_REPORT_URI + " /myrelativeuri"
I usually prefer the tests as short as possible. Maybe you're testing
that these headers don't interfere with the CSP_HEADER_FIREFOX also?
!!!> Yes
* Added a new test case (see: test_provides_csp_features_no_case02)
where this CSP is provided:
# Note the errors in the directive:
# default-src -> default-source
# img-src -> image-src
header_value = "default-source 'self'; image-src *"
And then tested provides_csp_features, which in my opinion should
return False (because the browser will ignore it) and returned True.
!!!> Code fixed to take this case in account
* See also test_provides_csp_features_no_case03
!!!> Code fixed to take this case in account
!!!> All Unit tests pass using the last repository update from today.
To sum up, great work on the utils and tests, I'm very happy with this
code Regarding the next steps:
* retrieve_csp_report_uri could be easily integrated with w3af's
code in order to extract URIs from those headers and make them
available to other plugins to find vulnerabilities in them. This
should be done in core.data.requests.factory (there is similar code
there for handling 302 redirects). Do you want to give it a try?
!!!> I will work on this point
* The code we have up to now is a great parser for CSP, but lacks
high level functions/methods to easily use in other places of the
framework. In the future I would like the xss.py code to look
something like this:
if vulnerability_found(response):
if correctly_implements_csp(response):
msg = 'Your site is vulnerable to XSS but this
vulnerability is mitigated because you correctly implemented CSP, only
old browsers will be affected by it'
else:
msg = 'Your site is vulnerable to XSS'
Do you think that it would be possible to achieve this kind of
abstraction? Could you add it to the utils.py module and test it?
!!!> I will work on this point
Thanks for your contributions and sorry for the delay.
!!!> You're welcome, it's up to me to thanks you to give me the
opportunities to contribute to this great project ;o)
PS: Gtalk to me if you've got a minute to talk about this
!!!> My Github repository is up to date with my update....
Best regards,
Dom
------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
_______________________________________________
W3af-develop mailing list
W3af-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/w3af-develop
