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