Perl provides several functions for list manipulation. For instance, the map() function takes an expression or block, applies it to each element in a list, and returns the list of mapped elements. If it is given a block, the block is executed with $_ assigned to each element of the list in turn. The perlfunc manpage adds:

Note that $_ is an alias to the list value, so it can be used to modify the elements of the LIST. While this is useful and supported, it can cause bizarre results if the elements of LIST are not variables. Using a regular "foreach" loop for this purpose would be clearer in most cases.

Although it is supported, using map() to modify a list in place can lead to surprises in maintainability and is thus forbidden.

Many other list functions provide similar functionality, using a block on various list elements. The grep() function is one such example, as are the first() and reduce() functions in List::Util and all of the functions in List::MoreUtils.

Finally, the sort() function also provides aliases to its comparison blocks, so a comparison block for sort() must not modify its variables.

Noncompliant Code Example (grep())

This noncompliant code example reads the /etc/passwd file and lists all users who use /bin/sh as their login shell.

open( PASSWD, "<", "/etc/passwd") or croak "error opening /etc/passwd: stopped"
my @users = <PASSWD>;
my @shell_users = grep +(s|/bin/sh||), @users;
foreach my $user (@shell_users) {
  print "Shell User: $user";
}

However, because the grep() block removes /bin/sh from any input line that contains it, it modifies the @users list so that no user has /bin/sh!

Compliant Solution (grep())

This compliant solution does the same thing but does not modify the @users array.

open( PASSWD, "<", "/etc/passwd") or croak "error opening /etc/passwd: stopped"
my @users = <PASSWD>;
my @shell_users = grep +(m|/bin/sh|), @users;
foreach my $user (@shell_users) {
  $user =~ s|/bin/sh||;
  print "Shell User: $user";
}

Compliant Solution (apply())

This compliant solution does the same thing but uses List::MoreUtils::apply(), which guarantees not to modify its input list.

open( PASSWD, "<", "/etc/passwd") or croak "error opening /etc/passwd: stopped"
my @users = <PASSWD>;
my @shell_users = List::MoreUtils::apply { s|/bin/sh|| } @users;
foreach my $user (@shell_users) {
  print "Shell User: $user";
}

Risk Assessment

Failure to handle error codes or other values returned by functions can lead to incorrect program flow and violations of data integrity.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

EXP34-PL

Medium

Likely

Low

P18

L1

Automated Detection

Tool

Diagnostic

Perl::Critic

ControlStructures::ProhibitMutatingListFunctions

Bibliography

[Conway 2005]"List Processing Side Effects," p. 114
[CPAN]Bar, Graham. List::Utils
[CPAN]Kennedy, Adam. List::MoreUtils
[Wall 2011]perlfunc

 


3 Comments

  1. Anonymous

    It should be mentioned that List::MoreUtils' apply function is designed not to modify the original, so

      @result = apply { $_ * 2 } @data

    does not modify @data.

    1. I've added this as another Compliant Solution.

      1. Anonymous

        it is usually used like this:

        use List::MoreUtils qw'apply';

        ...

        my @shell_users = apply { s|/bin/sh|| } @users;

        ...

        Also this doesn't return a list of only the modified elements like the original does.

         

        It is also possible to use map, but that tends to reduce clarity, if you are also using it as grep.

        my @shell_users = map { ($_) x!! s|/bin/sh|| } @users;