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 :)
* Code in test_utils.py did not respect the 80 column limit, applied
autopep8 to it to fix it
* 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";
...
* 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?
* 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.
* See also test_provides_csp_features_no_case03
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?
* 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?
Thanks for your contributions and sorry for the delay.
PS: Gtalk to me if you've got a minute to talk about this
Regards,
On Fri, Dec 14, 2012 at 10:04 AM, Andres Riancho
<andres.rian...@gmail.com> wrote:
> Damn! Forgot about this one, reading right now. Give me some mins.
>
> On Fri, Nov 30, 2012 at 12:16 PM, Andres Riancho
> <andres.rian...@gmail.com> wrote:
>> I'm on vacations until next Monday, I'll answer that day.
>>
>> On Fri, Nov 30, 2012 at 2:41 AM, Dominique Righetto
>> <dominique.righe...@gmail.com> wrote:
>>> Andres,
>>>
>>> I have implemented all your remarks and I have aligned the "utils.py" code
>>> to stick to 80 columns using the Python official style guide recommendation.
>>>
>>> I have executed my unit tests against the revision 6177 of Threading2 branch
>>> (last from today) and all unit tests pass.
>>>
>>> The github repo is up to date with source code.
>>>
>>> Thanks in advance for your review ;o)
>>>
>>> Dom
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Keep yourself connected to Go Parallel:
>>> TUNE You got it built. Now make it sing. Tune shows you how.
>>> http://goparallel.sourceforge.net
>>> _______________________________________________
>>> W3af-develop mailing list
>>> W3af-develop@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/w3af-develop
>>>
>>
>>
>>
>> --
>> Andrés Riancho
>> Project Leader at w3af - http://w3af.org/
>> Web Application Attack and Audit Framework
>> Twitter: @w3af
>> GPG: 0x93C344F3
>
>
>
> --
> Andrés Riancho
> Project Leader at w3af - http://w3af.org/
> Web Application Attack and Audit Framework
> Twitter: @w3af
> GPG: 0x93C344F3
--
Andrés Riancho
Project Leader at w3af - http://w3af.org/
Web Application Attack and Audit Framework
Twitter: @w3af
GPG: 0x93C344F3
------------------------------------------------------------------------------
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
