Task-Focused PHP Code Review How-To

The beginning of documenting task-focused PHP code review activities

Introduction to the Eclipse debugger and the PHP vs Debug perspectives

Identify sources and sinks

We need to find places where user-submitted input is processed by requsted pages, and evaluate how it’s used by the application in making security-relevent decisions, or when it’s passed to functions which are useful for attackers to abuse.

In practice, this means searching the for page parameter inputs, using patterns such as

$_GET
$_POST
$_REQUEST
$_SERVER

$_SERVER returns an array that usually contains user input, see http://www.php.net/manual/en/reserved.variables.server.php

Testing in Eclipse

Open the Preferences window, Window -> Preferences.
In the list of settings categories, choose PHP -> Editor -> Syntax Coloring

Then, in the Synax Element: list, click “Superglobal Variables,” then make sure Enable is checked. Select Bold, then set the color to something easily noticable, that you can’t miss, such as red, or hot pink.

Now, for every file I open and review, every time $_GET, $_POST, etc are read, they will be  visually distinct from the rest of the code. It’s hard to miss bold, bright pink text.

I also like to set Varisble to a dark grey, vs their default of black, as they then become more readily apparent as you visually scan the code.

Identifying Insecure Use of Superglobals

It usually looks something like this:

if isset($_GET[‘foo’]) && $_GET[‘foo’] == ‘something-or-another’){

// etc etc

}

Automated

Existing OSS tool? Script a new Eclipse plugin to grep the workspace for them, and report findings in a list?

Forcible Page Browsing

Some pages were intended to be included by another page, not called directly. Typically, they contain classes, functions, and methods which are means to be used by other pages.

Sometimes, these pages contain code which, when the page is directly called, processes user input, sets session variables, client cookies, outputs debut information, performs administrative functions, or does other things which might be interesting from a security perspective. Functions and classes don’t execute unless called directly. Code * between * functions and dashes will, however.

Testing in Eclipse

Check the first file:

In Eclipse, you can right-click the gutter, then click Folding -> Collapse All. Now, you should be able to easily see what’s left outside of code blocks, and which will execute when a page is directly called by a browser.

Next, test up the stack: Any other file called with either require or include that you can now see, which is mentioned outside of a function or a class, needs to be similarly verified.

Now, test down the stack: We need to similarly look at other pages which include the one we’re looking. It’s possible that another page could include this one, and use included functions from the page you started looking at, insecurely.

Make sure to carefully check for external input which will get processed – stuff from the sources/sinks list above, from a database, from a web service, etc. Sometimes you’ll see things such as code which only runs when a HTTP get flag is set, or a cookie is set ot a hardcoded value. This is a good opportunity to set a breakpoint in eclipse, load the page with the appropriate variables set, and step through the code to see where and how security decisions are being made, and how you might be able to affect them.

Identifying Function Callers

While reviewing a file and coming across functions which perform security decisionmaking, or otherwise catch your attention, it is often necessary to identify where in the application this function is called from.

Testing in Eclipse

First, click the function name, it should highlight automatically. Then, right-click, and click ‘Search.’

In File Search, the highlighted name should already be there. Eclipse will search the worksdpace, looking for places that reference the function name.

Breakpoints – where, how, why, when testing

Review Console Output tab

Review Tasks tab

How is that implemented? Using Open Declaration

Search tips

RATS and Eclipse

Please post comments/suggestions to on how to review PHP code for vulnerabilities here.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s