HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Security feedback sought on Perl code running command on unsafe filename in Unix env

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unixfilenameperlunsafesecurityrunningcodesoughtfeedbackcommand

Problem

I'm looking for security feedback on the following fully functional code.

The code is trying to safely use the Unix 'file' command to give details about the file.

A hard link is used to create a safe filename that can be used in a command line.

The paramref is a hash read from config, and considered safe. file_base_dir is a directory with only alphanumerics in the name. The 'file' command is the standard one.

What ways could my code be exploited?

sub file_details {
    my ( $self, %arg ) = @_;
    my $filename = $arg{'filename'};
    my $paramref = $arg{'paramref'};
    my $file_cmd = '/usr/bin/file';

    my $safe_filename = $paramref{'file_base_dir'} . "/link_to_unsafe_file_file_type_build";

    # Just in case the link exists.
    unlink($safe_filename);
    if ( not link( $filename, $safe_filename ) ) {
        confess "Failed to create link $safe_filename to $filename because $!";
    }

    my $file_type = `$file_cmd '$safe_filename'`;

    # remove the link
    unlink($safe_filename);
    $file_type =~ s/\A[^:]+://x;
    chomp $file_type;

    return $file_type;
} ## end sub file_details

Solution

Interesting approach with link.
If the only vulnerable parameter is filename,
this technique protects from malicious input,
because the filename parameter will not be part of the `... bit executing shell commands.
I don't think it's possible to trick
link into command execution.

On the other hand, although you say that "file_base_dir is a directory with only alphanumerics in the name",
this function doesn't ensure that, simply trusts it to be true.
It would be good to add a call in this function to another function that validates the directory parameter.

I tried to think of other ways to validate the
filename parameter,
without creating a link.
You could forbid certain characters such as
; and $ to prevent command injection,
but I'm not sure that's an exhaustive list.
So while the
link trick seems hackish,
it does give comfort that vulnerable parameters never participate in a
...` shell expansion.

Context

StackExchange Code Review Q#82041, answer score: 3

Revisions (0)

No revisions yet.