[wp-hackers] WordPress plugin inspections

[Harry Metcalfe]

Hi Simon,

Thanks for your reply and the kind words.

I would love to be able to make these reviews more thorough, but we're 
subject to the same commercial realities as any other company: our 
clients want these reviews, and don't want to pay for them. So we're 
doing our best to make them as good as we can, within the resources we 
have. I'm definitely open to ways we can make them better and have 
already made some changes to these pages based on the feedback in this 
thread.

I've had a read of the Relevanssi one, and I agree that it's not quite 
right at the moment. It's kinda judgemental and the server hack thing 
doesn't have enough context. It's got Glyn as the author but I'm pretty 
sure I rewrote that one, so it's my bad. We're still quite early on in 
the process and we will get better.

I've edited those findings to make them a bit more objective. Here's the 
original one, QFT:

> This plugin takes an idiosyncratic, dangerous approach to SQL 
> generation. This plugin contains a large number of long and 
> complicated SQL queries and there is no organised or methodical 
> approach to generating them safely.
>
> We have sampled a quite a few of these queries and none appear to be 
> injectable, but we suspect this is more likely to be due to luck than 
> good judgement.
>
> This plugin also has a history of broken releases, including one which 
> contained malicious code added to the distribution after the author's 
> website was hacked in July 2013.
>
> Tread cautiously.

I do stand by the rating, though. This plugin has not "failed" (there 
isn't really any such outcome). It's assessment is "use with caution" 
and I think that is right.

That the author does appear to be safely escaping queries without 
preparing them saves it from being rated "potentially unsafe", and the 
lack of systematic SQL preparation is an issue. This plugin is more 
likely than something systematic to have a dangerous query someone has 
overlooked, or to introduce one in the future. And that likelihood is 
made more significant by the very complex query generation in functions 
like relevanssi_search in search.php, where the generation of one query 
is spread over 100+ of lines of code.

The reviews are aimed at anyone running a WordPress website. We've tried 
to strike a balance between adding enough technical detail to the 
findings for developers and security people to follow them up, and 
having a clear recommendation, so that non-technical users gain some 
benefit as well.

Harry


On 20/02/2014 00:14, Simon Blackbourn wrote:
> Hi Harry
>
> I think there is potentially a very useful idea and service here, but I
> really think a lot more care, depth, clarity (and possibly right to reply
> for plugin authors) needs to go into the reviews.
>
> Here's an example that really stands out to me:
> https://security.dxw.com/plugins/relevanssi-premium
>
>
> "This plugin also has a history of broken releases, including one which
>> contained malicious code added to the distribution after the author's
>> website was hacked in July 2013."
>
>
> I was the person who discovered that Mikko's website had been hacked and
> the resulting attempted malicious code that was inserted into his plugin (I
> say attempted, because due to a typo by the hacker it didn't actually do
> anything). I reported it responsibly by emailing him privately at 11.30pm
> on a Friday night. By the Monday he had fixed it, released a new version,
> installed additional security measures on his server and emailed all his
> users to openly explain and apologise for what had happened, which is
> pretty much a textbook exemplary way to deal with this sort of thing.
>
> It seems unfair therefore that you have turned something that happened
> seven months ago that was resolved so speedily and responsibly into a very
> public black mark against this plugin, especially in a format that doesn't
> give the author any right to reply.
>
>
> "We have sampled a quite a few of these queries and none appear to be
>> injectable, but we suspect this is more likely to be due to luck than good
>> judgement."
>
>
> What you're saying here is you conducted an incomplete test, couldn't find
> anything wrong, yet you then decided that this must be luck, so you're
> going to count it against the plugin anyway?! The word 'suspect' really
> shouldn't have any place in a professional and public vulnerability review
> - either you test fully and find a vulnerability (which you then report in
> the proper manner to the author) or you don't.
>
> You've then failed the plugin on three code-related criteria, but then
> state in the box on the right that you haven't actually done a proper code
> inspection.
>
> Finally, it's very unclear to me who the reviews are aimed at? If it's for
> non-teccie end users, then they are very unlikely to understand concepts
> such as "unprepared SQL statements", but as a developer, an incomplete high
> level review that hasn't delved comprehensively into the code is no use to
> me at all.
>
> All the best
> Simon
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers