Welcome screen sanitizing #131

Closed
justintadlock opened this Issue Jan 10, 2017 · 1 comment

Projects

None yet

3 participants

@justintadlock
justintadlock commented Jan 10, 2017 edited

In inc/admin/welcome-screen/welcome-screen.php, make sure to sanitize $_GET['id']:

$action_id = ( isset( $_GET['id'] ) ) ? $_GET['id'] : 0;

I noticed that it's escaped immediately after when being echoed, which is fine. However, it's later used without being sanitized. absint() should be fine.

One line 136, I'd also sanitize this to be on the safe side:

switch ( $_GET['todo'] ) {

Side note: Your switch statement should have a default.

On line 327, I'd sanitize this. While it doesn't seem to be a security issue because it's escaped when it's in use, I like to sanitize when I first grab the variable.

$active_tab   = isset( $_GET['tab'] ) ? $_GET['tab'] : 'getting_started';

You should also wrap each of these in wp_unslash() before sanitizing. This is really unnecessary in these specific cases (no ' or " marks that would be slashed), I do know for a fact that the future version of the .ORG theme check will hang up on this (something I just found out about in the last couple of days). So, while unnecessary in some cases, it's good practice and will eventually be required.

@cristianraiber cristianraiber modified the milestone: Shapely 1.1.0 Jan 11, 2017
@cristianraiber
Collaborator

Fixed in: 188b1d6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment