Thanks for the code review, really appreciate it. I've attached the updated
file and made some inline comments:

* Line 2 incorrectly says "crossDomainXMLChecker.py"
>

Oops, that one escaped my notice after copy/paste.


> * Remove "_testCheck"
>
>
Done. Another brain fart.


> * Might be a good idea to move "self._themeList = {" to a different
> file if we plan to make it grow.
>
>
Good idea, I'm just not sure where to put it in the framework? Under
core.data.constants.wordpressThemes.py maybe?


> * I would only set "self._exec" to false after identifying a valid
> wordpress base. It might be the case that in the first run of the
> plugin the base is not found but then because of enabling webSpider +
> wordpress_themeauditor in the second call to the wordpress plugin
> (with a different URL) the blog is found.
>
>
Thanks, that makes sense, changed.


> * What if the site is https? response.getURL().url_string[7:] , there
> should be a method in urlParser.py that returns what you need without
> depending on [...]
>

You're right, there is. I remember looking through it briefly when I wrote
this and must have missed it. Replaced with response.getURL().allButScheme()


> * Check if this is done somewhere else in the other discovery plugins
> we have and either improve the existing one or replace it with yours
> if it is far better :)
>
>
I don't think any other plugins check wordpress themes. I looked through
the existing wordpress plugins and did a few greps of the plugins directory
and came up with nothing. Hopefully someone can correct me if I'm missing
something here.


> * Maybe this regex is too open and could generate false positives?
> regex_str = 'Log\s*In\s*<\s*/title\s*>'
>
>
Agreed, it is very open. I thought it would be ok since it will only check
that regex if the page is called wp-login.php. Even still I suppose you
could end up with some sort of 401 page where the URL would not be modified
and this regex would match. I've updated to to be a little more selective
to wordpress login pages, it now looks for the string 'wp-admin'.

* This doesn't look good,
> response.getURL().getDomainPath().url_string[:-1].rpartition('/')[2] ,
> all the hardcoded [:-1] and [2] tend to crash with special cases
>
>
Fixed with a more elegant solution, the theme name is no longer pulled from
the url, just passed in to the function


> * In "def _checkStyleCss(self,response):" and in order to make it
> stronger and don't depend only on the response not being a 404, what
> about looking for a very common string like "size" or "font" or
> something like that?
>
>
I've got a regex to check if it's actually returning CSS. Didn't want to go
with a common string just because you never know how someone might write
their CSS. It is (?:\s*\S+\s*{[^}]*})+. It seems to work well.


> * getLongDesc() returns info from crossdomain.xml
>
>
Fixed.


> Except from those small details, it looks very good! What do you
> think, should it be merged into our pre-existing wordpress plugins or
> should it live as an independent one? If so, can we relate your plugin
> with the others using "getPluginDeps()" ?
>
>
I like the idea of merging it with the wordpress_fingerprint plugin. I
don't think it makes sense to have it separate since it really is just
doing some more wordpress fingerprinting and having a lot of plugins that
all do small wordpress specific tasks seems like a waste.


> >
> ------------------------------------------------------------------------------
> > Live Security Virtual Conference
> > Exclusive live event will cover all the ways today's security and
> > threat landscape has changed and how IT managers can respond. Discussions
> > will include endpoint security, mobile security and the latest in malware
> > threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> > _______________________________________________
> > 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
>

Attachment: wordpress_themeauditor.py
Description: Binary data

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
W3af-develop mailing list
W3af-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/w3af-develop

Reply via email to