Static Code Analysis with Perl::Critic

It is very important for any group that develops software to adhere to a set of software coding standards.  These coding standards not only help to improve the readability and maintainability of the code base, but can also help to improve the security of the resultant applications.  Any set of coding standards should describe a set of best practices for dealing common types of coding issues. One way to help ensure that code follows the standards dictated by industry and organizational best practices is to make use of a static code analyzer.  For Perl, a popular static code analyzer is the Perl::Critic module.  Before we take a look at what this module does, let’s purposely create some bad Perl code as a test case and save it as badperl.pl.  

#!/usr/bin/perl

$string='string';

#assume $filename provided by user
open (STATFILE, "/home/user/$filename");

#assume input provided by $user
system("cat $input");

 

Now that we have this example of some really bad Perl coding practices lets create a few line of Perl to run our script through Perl::Critic.

use Perl::Critic;
  
my $file = 'badperl.pl';
my $critic = Perl::Critic->new();
my @violations = $critic->critique($file);
print @violations;

If we execute the above we will end up with output as follows:

chris@chris-Aspire-5742:~/Desktop$ perl critic.pl
Code before strictures are enabled at line 3, column 1. See page 429 of PBP.
Bareword file handle opened at line 6, column 1. See pages 202,204 of PBP.
Two-argument "open" used at line 6, column 1. See page 207 of PBP.

 

The Perl::Critic module tells of several problems with the first being that “use strict” was not made use of in our badperl.pl script.  Next, it informs us of some problems with the open statement as well.  The form of open used in that sample is a depreciated syntax which makes use of global typeglobs (the Bareword error), rather than a more modern lexically scoped file handle.  Perl Critic also points out that the older "two argument" open syntax is used rather than the newer "three argument" syntax which can prevent some potential issues with file names that contain special characters and thus improve the stability and reliability of the code.  Using a static code analyzer is a strongly recommended practice and in the majority of cases will help ensure an improvement in the overall quality and consistency of the code, even though compliance with policies may not always be 100% possible.  

 

It is important to note however, that static code analyzers are very useful tools, but that they are not able to catch every possible coding mistake.  In particular design related errors in the code will not be picked up by static code analysis.  For example, in badperl.pl the $filename is never validated and this may provide a potential vector for a directory traversal type attack.  Likewise in the system call, prior validation is also lacking and could allow a user to provide something like “/etc/passwd” as $input.  These issues however were not raised by this analysis, however.  Remember that passing a static code analysis does not necessarily equate with secure code.