All Perl subroutines may be used in an expression as if they returned a value, but Perl subroutines are not required to have an explicit return statement. If control exits a subroutine by some means other than an explicit return statement, then the value actually returned is the last value computed within the subroutine. This behavior is never intended by the developer (else she would have added a return statement) , and may potentially be information that is private to the subroutine.
...
| Code Block | ||||
|---|---|---|---|---|
| ||||
sub subroutine {
# ... do stuff
1; # always returns 1
}
|
...
| Code Block | ||||
|---|---|---|---|---|
| ||||
package Bank;
# ...
sub deposit {
my ($amount, $account, $pin) = @_;
my $good_pin = _get_pin( $account);
if ($pin == $good_pin) {
my $balance = _get_balance( $account);
_set_balance( $account, $amount + $balance);
} else {
my $failed = $good_pin;
}
}
|
The deposit() function does not explicitly return any value. Consequently, if any code invokes the deposit() routine , and does something with the return value, what value does it actually receive?
The answer is, the last value actually computed within the deposit() routine will be used as the return value. But to determine the last computed value, you have to do some control flow analysis. For this routine, if a valid $pin is supplied, then the last value computed is the return value of _set_balance(), so invoking deposit() with a valid PIN yields the result of the private _set_balance() routine, which may be sensitive.
...
This compliant solution adds a trivial return statement to the function. Now the function always returns undef, rather than any sensitive information.
| Code Block | ||||
|---|---|---|---|---|
| ||||
sub deposit {
my ($amount, $account, $pin) = @_;
my $good_pin = _get_pin( $account);
if ($pin == $good_pin) {
my $balance = _get_balance( $account);
_set_balance( $account, $amount + $balance);
} else {
my $failed = $good_pin;
}
return;
}
|
...
EXP01:EX1: A function need not have an explicit return value if it is only used in a program that never requests its return value.
Risk Assessment
Attempt An attempt to read the return value of a function that did not return any value can cause data encapsulated by the function to leak.
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
EXP01-PL | info Medium | likely Likely | low Low | P18 | L1 |
Automated Detection
Tool | Diagnostic |
|---|---|
Perl::Critic | Subroutines::RequireFinalReturn |
Bibliography
...
| [Conway 2005] | "Implicit Returns," p. 197 |
|---|---|
| [CPAN] | Elliot Shank, Perl-Critic-1.116 |
...
...
|http://search.cpan.org/dist/Perl-Critic/lib/Perl/Critic/Policy/Subroutines/RequireFinalReturn.pm]EXP30-PL. Do not use deprecated or obsolete functions 02. Expressions