800812 - SecReview: Update Mozilla Hacks WordPress Editorial Calendar plugin

Status: VERIFIED FIXED

(mozilla.org :: Security Assurance: Review Request, task)

Description

[Robert Nyman]

More info at https://hacks.mozilla.org/wp-admin/plugins.php?plugin_status=upgrade

Comment 1

[Matt Fuller :mfuller]

I'm assigning Stefan to this bug, as it looks like a good first WP review for him to do.

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Robert, just to be sure, this is about the following plugin right?

http://wordpress.org/extend/plugins/editorial-calendar/

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I reviewed version 2.5

In the PHP code my only worry is the edcal_echoFile() function, which states "This is a utility function to open a file add it to our output stream." It seems however that this is only used with hard-coded file names that are relative to dirname(__FILE__).

The AJAX calls are protected by a nonce (csrf token) so that is good.

The app only does queries on posts using the the provided Wordpress APIs.

It does not execute shell commands. It does not reimplement Wordpress functionalty and instead uses wordpress provided functions.

Most of the work is done in JavaScript.

The JS code has unit tests, which "It is only loaded if you add the qunit=true parameter to the URL for the calendar." I would recommend to simply delete the edcal_tests.js file completely from the deployment

The lib directory contains a lot of JS libraries. Might be a good idea to remove all libraries that are not used for this deployment. The following are optional and not used in a default install I think: (see edcal_scripts()) jquery.bgiframe.js, humanmsg.js, jquery.timepicker.js, tools.scrollable-1.1.2.js, tools.scrollable.mousewheel-1.0.1.js, json2.js

The admin UI (The new Calendar option under Posts) does its work by making AJAX calls to the PHP code. All calls are CSRF protected (Wordpress calls it a reqyest nonce) so it is very difficult to redirect a malicious link to this plugin.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Using ZAP I have tried to forge requests to the admin AJAX code to post new entries. The code seems to correctly check the nonce and cookie (session) and does not allow me to make changes without being properly logged in.

Comment 5

[Robert Nyman]

Thanks for evaluating!
This plugin isn't that important to the Hacks blog, so if it has issues, we don't have to use it.

Just let me know what you prefer.

Thanks!

Comment 6

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Hi Robert, I think it is all good. This is my first review so I just need to follow up with my colleagues on how to officially sign off on it tomorrow.

Comment 7

[Robert Nyman]

No worries, I appreciate the dedication and doing it properly! :-)