Dom,

On Sat, Nov 10, 2012 at 9:04 AM, Dominique RIGHETTO
<dominique.righe...@gmail.com> wrote:
> Hi,
>
> About CSP plugin and according to Andres remark i have started working on an
> utility module for CSP header values parsing.
>
> I have implemented a utility module with associated unit tests (i have taken
> the CORS utility module from Threading2 branch as reference).
>
> I'm still working on it and it's the first step in my work to implement an
> CSP Grep plugin...
>
> Sources are published here :
> https://github.com/righettod/w3af-contribs/tree/master/core/controllers/csp

I'm very glad you did it this way since I already found a place where
we'll end up using the CSP parser: xss.py. Right now our XSS detection
engine doesn't take CSP into account, so even if the site has CSP and
"echoes user input" we'll tell the user that he has XSS vulnerability.
When this is finished, we'll integrate the CSP module intro the XSS
plugin too :)

* Code looks much nicer than previous contributions, congrats on that :)

* Please try to respect the 80columns limit for each line

* Most of this code:

"""
    for header_name in headers:
        header_name_upperstrip = header_name.upper().strip()
        process_header = False
        #Define header processing condition according to
"select_only_reportonly_policies" parameter value
        if not select_only_reportonly_policies:
            if header_name_upperstrip == CSP_HEADER_W3C.upper() or
header_name_upperstrip == CSP_HEADER_FIREFOX.upper() or
header_name_upperstrip == CSP_HEADER_CHROME.upper() or
header_name_upperstrip == CSP_HEADER_IE.upper():
                process_header = True
        else:
            if header_name_upperstrip == CSP_HEADER_W3C_REPORT_ONLY.upper():
                process_header = True
"""

    Can be replaced by using the "iget" method that I added recently
to the Headers class. So, instead of looping and doing all those
upper(), you just do:

"""
header_value, header_name = headers.iget('Referer', None)
"""

    Check the method documentation and unittesting for examples.

* Without knowing much about CSP I ask: "Is splitting by ; ok?"

   directives = directive_list.split(";")

Can we have header a value like: a;b;"c;d" ?

* It's a very stupid detail which I have to tell you cause of my OCD
;) Instead of doing "if len(directive_strip) > 0:" and having the rest
of the code be there with one extra unnecessary indent (unnecessary
because there is no else) you could do something like:

if len(directive_strip) == 0:
    continue
<rest_of_code_goes_here>

    Same thing with "if len(parts) >= 2:"

* Loved the unittests, very complete!

* In order to use this in the XSS detection plugin, do you think it
would be possible to add some shorthand method in the utils.py file
that  returns True if the CSP is configured to allow unsafe-inline?
Something simple to do I guess, it would be a method called
unsafe_inline_enabled() which takes the headers as parameters.

    All in all, very good job, let me know when you complete these
comments and we'll put it into the SVN.

Regards,

> Have a nice day :)
>
> --
> Cordialement, Best regards,
> Dominique Righetto
> dominique.righe...@gmail.com
> dominique.righe...@owasp.org
>
> Twitter: @righettod
> GPG: 0xC34A4565323D19BA
> http://righettod.github.com
> "No trees were killed to send this message, but a large number of electrons
> were terribly inconvenienced."



--
Andrés Riancho
Project Leader at w3af - http://w3af.org/
Web Application Attack and Audit Framework
Twitter: @w3af
GPG: 0x93C344F3

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_nov
_______________________________________________
W3af-develop mailing list
W3af-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/w3af-develop

Reply via email to