Taras,

On Mon, Apr 23, 2012 at 7:58 AM, Taras <ox...@oxdef.info> wrote:
> Andres,
>
>
>>     Could you please explain me this comment? "# TODO need to check
>> here for auth cookie?!"
>
>
> Simply, does web app vulnerable to ClickJacking if it hasn't user's area?
> In other words do we need here to check auth state like in CSRF detection?
> In ideal case we need it.

Ahhh! Now I understand, you're right about that TODO, we should leave it there.

>
>>     Instead of the following:
>>
>> 49              headers = response.getLowerCaseHeaders()
>> 50              for header_name in headers:
>> 51                  if header_name == 'x-frame-options'\
>> 52                          and headers[header_name].lower() in ('deny',
>> 'sameorigin'):
>> 53                              return
>>
>>     You could do something like:
>>
>> headers = response.getLowerCaseHeaders()
>> x_frame_options = headers.get('x-frame-options', None)
>> if x_frame_options and x_frame_options in ('deny', 'sameorigin'):
>>     return
>
> Agree, will fix it.
>
>
>>     This is actually not true:
>>
>> 76              if self._total_count == self._vuln_count:
>> 77                  msg = 'The whole target '
>> 78                  msg += 'has no protection (X-Frame-Options header)
>> against ClickJacking attack'
>>
>>     If we analyze 5 (self._vuln_limit = 5) and those 5 don't have
>> protection, that doesn't mean that all don't implement it.
>
> Agree, but please re-read this piece of code. There is no self._vuln_limit
> checks here.

Yes, but self._vuln_count size strictly depends on self._vuln_limit

>
>>     I would completely remove "self._vuln_limit" as it doesn't make
>> logical sense to only analyze "a section of the application" if we can
>> analyze all of it. Also, by removing "self._vuln_limit" you'll see
>> that the memory usage of "self._vulns = []" will grow linearly with
>> the application's size (if there is no protection) which is no good,
>> so I recommend using a temp_shelve.
>
> Hmm, the aim of self._vuln_limit is mostly to make report easy to read. If
> 15 or more requests are vulnerable do we need to store and show all these 15
> requests to the user?

We're reporting two vulnerabilities:
    * All are vulnerable, in which case we don't even need to add URLs
since *all* are vulnerable
    * Some are vulnerable, in which case we should print all URLs in
the vulnerability description (annoying in some cases... but needed in
my opinion).

>
>>     Sorry if I'm being too strict, but I think we can do better than this
>> :)
>
> There is no problem with criticism for me :)
> --
> Taras
> http://oxdef.info



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

------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
W3af-develop mailing list
W3af-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/w3af-develop

Reply via email to