Exakat 1.0.11 review
Happy new year 2018 to you all, code reviewers. The final versions for Exakat in 2017 were small upgrades: we took advantage of the end of the year to remove some bugs, and fix some long waiting tasks. So, in the end, it is the most stable version of Exakat that starts the year.
We added or extended several analysis: support for PHP 7.2.1 and 7.1.13; New security analysis that check Sqlite3::escapeString() because it doesn’t support “; the “weird variable names” analysis to include triplet (like $qqq); and just like arguments, inherited variables that are unused are now reported. Let’s check Exakat 1.0.11.
Never used inherited variables
Sometimes, functions declares arguments, but forget using them. This creates bloat, because when those functions are used, some fake values must be given just to keep it running. So, Exakat already had the ‘Unused Argument’ analysis.
For functions, it is quite easy to understand that unused arguments are also dead code. The same reality is slightly different for methods: methods’s signature may be compelled by an interface or a parent class. In such case, the extra argument is required, even if unused. Indeed, it is a sign that the conception needs some review.
Closure behave just like function: no argument use is dead code. Though, closures also come with ‘inherited variables’, which are passed at creation time, with the ‘use’ clause. Those variables must be residing locally in the creating scope, and are then used at execution time, even if the initial variable isn’t available anymore.
<?php
// Closure with argument and inherited variable $closure = function($argment) use ($inherited) { return $argument + $inherited; }
// Closure with unused inherited variable $closure = function($argment) use ($inherited) { return $argument + 1; }
?>
Exakat now reports closure with unused inherited variables.
Sqlite3’s escapeString() doesn’t escape double quotes
When building a SQL query to use with SQLITE3, there are two main ways to pass values: by prepared queries, or by escaping. The first one is the recommended way, as it separates clearly the query from its values. This reduces the possibility of injection to zero.
Escaping is the older way, and it requires preparing the value to be enclosed in single quotes. Sqlite3 uses double quotes to enclose names, and single quotes to enclose values.
This is particularly surprising when coming from other SQL implementations, where single or double quotes may be used when it is convenient.
<?php $value = 'this includes " quotes'; // This will actually have a SQL query // This may end up being an injection $sql = 'SELECT * FROM table WHERE column="'.$sqlite->escapeString($value).'"'; $sql = 'SELECT * FROM table WHERE column=\''.$sqlite->escapeString($value).'\''; ?>
Next time, when relying on Sqlite3, always makes your values a bonded parameter, or a special value between single quotes. In between, Exakat will search for your SQL queries, and report any irregular usage.
Weirdly names variables
Alexandre Joly (https://twitter.com/bsmt_nevers/status/949238391769653249) tweeted about variables names made of one character, repeated as needed. This looked like a great idea to expand the weirdly named analysis.
Now, on top of classic typos, like $tihs or $_PSOT (all found in real life, several times), we also report variables with triplets, like $qqq or $aaaa.
Note that it lead quickly to some false positive : for example, $www and $YYYY. This last case is actually a borderline situation : YYYY basically means ‘year number on four characters’ and is used after a date() formatting. So, there is actually meaning in this naming schema, though it is not good to give a format’s name to a variable.
Check your code with newer PHP versions
As you know already, Exakat handles already PHP 7.3, which is coming up with new syntactic sugar, like a final empty argument when calling functions. We keep an eye on the RFC PHP wiki to keep track of the new features.
In the mean time, PHP keeps producing new versions, such as 7.2.1 and 7.1.13, thanks to Sara Golemon and Remi Collet. Anytime you want to know if it impacts your code, you can check the ‘bugfixes‘ report. This report check any function, extension and PHP feature in your code, and check the bug fixes database to see if the underlying code has changed.
For example, gethostname() has been fixed for host name longer than 64 characters. If you’re using gethostname(), you’ll be happy to be safer with those. If you haven’t noticed anything yet, may be you can add a test or two just to be safe.
Happy PHP code reviews
Exakat 1.0.11 is the most stable Exakat version. We have reached over 4650 unit tests recently, and we’ll keep adding more till the next version. For that, we keep on auditing Open Source project (and exakat itself), searching for false positive that may be eradicated. If you find any in your code, join on the slack channel, or send us a tweet.
All the 320+ analyzers are presented in the docs, including the shy ‘Use pathinfo() Arguments‘ : pathinfo() has a second argument to select only useful data. Download Exakat on exakat.io, upgrade it with ‘exakat.phar upgrade -u’ and like us on github: https://github.com/exakat/exakat.