Some new features

Last Friday, I had a conversation with nEUrOO in #stratego about the results he was having with php-sat (see his blog for more details). After the conversation, and after looking at the test-file he mentioned in his blog, I got started and added 2 new features to php-sat.

The first new feature is aimed at the usability of the tool itself, and is visible by a new command-line option: -ra CODE. This option accepts one of MCV, COR, EI, STY or OPT as input, and makes sure that the only analysis that is run is the one that belongs to the given code. In other words, you can now run, for example, only the correctness checks by calling php-sat with: --ra COR. This gives you a somewhat coarse-grained control over the behavior of the tool. A plan for more fine-grained control (on the level of patterns) is also mentioned some time ago, but the implementation of this level of control requires some more thoughts. In the meantime, please enjoy running a single kind of bug-patterns :).

A second new feature is added to the analysis of safety-levels. Consider the following example:
<?php
echo addslashes(htmlentities($_GET['name']));
?>
The default configuration for echo requires a parameter to have both the level EscapedHTML as well as EscapedSlashes. Furthermore, the default configuration defines the return-type of the functions as:
function: addslashes       level: escaped-slashes
function: htmlentities level: escaped-html
So this piece of code should not be flagged by php-sat. Unfortunately, previous revisions did flag this piece of code!

The problem here is that the analysis uses the safety-level of a function that is mentioned in the configuration file without considering the parameter of the function. This behavior works well for most functions, but when functions only add a property to their parameter it becomes incorrect. Because of this behavior, the echo-statement is flagged because the call to addslashes is only annotated with EscapedSlashes.

Fortunately, the solution is not that complicated. Since we know that there are several functions that add a certain property to their parameter, we add the possibility to specify this in the configuration file. The new syntax for functions that add a safety-level is a '+' after the level of the function. This '+' forces a function to inspect the level of its (only) parameter and combine this with the level that is specified as its safety-level. The combination of the levels is the result of the function call. (Small note: this behavior is (currently) only supported for functions with a single parameter.)

So from now on, when the following configuration is used:
function: addslashes       level: escaped-slashes +
function: htmlentities level: escaped-html +
the example above is not flagged anymore because the call to addslashes is annotated with its own safety-level (EscapedSlashes), as well as the safety-level of its parameter (EscapedHTML). A pretty useful feature I would say.

5 comments:

nEUrOO said...

Good :) I'm waiting for testing this!
Another question, is there any documentation about bugpatterns? Do you think it's possibnle to make some on SQL syntax for SQL Injection

Eric Bouwers said...

Good :) I'm waiting for testing this!
Can't wait to see the results ;)

Another question, is there any documentation about bugpatterns?
Currently, the only documentation about the bugpatterns is in the repository:
https://svn.cs.uu.nl:12443/repos/psat/php-sat/trunk/doc/
Each kind of bugpattern has its own directory containing one file per bugpattern.

Do you think it's possibnle to make some on SQL syntax for SQL Injection
How would you see this? Analyzing the actual string passed to a function like mysql_query?

nEUrOO said...

> How would you see this? Analyzing the actual string passed
> to a function like mysql_query?

exactly...

Eric Bouwers said...

exactly...
This is already happening in the MaliciousCodeVulnerability-pattern. By configuring functions like 'mysql_query' as a sensitive sink you can check whether the arguments to these functions are escaped properly.

I (currently) cannot think of other bugpatterns for SQL-queries, but if you have any ideas then please share them :)

nEUrOO said...

Hum cannot think about something else