How can I avoid escaping by accident in Perl using system()?

Question

I want to run some commands using the system() command, I do this way:

execute_command_error("trash-put '/home/$filename'");

Where execute_command_error will report if there was an error with whatever system command it ran. I know I could just unlink the file using Perl commands, but I want to delete stuff using trash-put as it's a type of recycling program.

My problem is that $filename will sometimes have apostrophes, quotes, and other weird characters in it that mess up the system command or Perl itself.

Answer 1

Generate the command name and arguments as an array, and pass that to system:

my(@command) = ("trash-put", 'home/$filename');
system @command;

This means that Perl does not invoke the shell to do any metacharacter expansion (or I/O redirection, or command piping, or ...). It does mean it does exactly what you told it to do.

sub execute_command_error
{
    system @_;
}

---

Borrowing information from the copious collection of comments:

Which is clearly documented in perldoc -f system or at perldoc.perl.org/functions/system.html (@Ether).

(See also the discussion of 'exec' below which is closely related.)

Did you mean to put $filename in single quotes? (@mobrule).

I did intend to use single quotes - I'm demonstrating that the $filename does not get expanded by Perl or Shell...In my test script, I used 'my.$file', and that gave me a file with a $ in the name - as I intended.

I think the desired quoting if you do want to invoke the shell (for example if you want some piping) is $command_line = "\"$command\" \"$arg1\" \"$arg2\"...". (@Jefromi).

Adding double quotes around the arguments won't help with embedded $, backtick¹, '$(...)' and related notations. You more nearly need single quotes around things, but then you need to rewrite embedded single quotes as "'\''" which generates a single quote to terminate the current single-quoted argument, a backslash-quote combination to represent a single quote, and another single quote to resume the single-quoted argument.

This would be a good solution if I used system command directly; however I am using webmin's execute_command function, which is a bit over my head so I wouldn't know how to edit it to allow for arrays. Could you expand on the rewrite of embedded single quotes as "'\''"...This is what I will use, for now. (@Brian)

Roughly speaking, the way the (Unix) shells treat single quotes is "everything from the first single quote up to the next is literal text, no metacharacters". So, to get the shell to treat something as literal text, enclose it in single characters. That deals with everything except single quotes themselves. As my comment says, you have to use the 4-character replacement string to get a single quote embedded into the middle of a single quoted argument.

There is probably a neater way to do it than this (using one or two map operations, perhaps), but this should work:

for (my $i = 0; $i < scalar(@command); $i++)
{
    $command[$i] =~ s/'/'\\''/g; # Replace single quotes by the magic sequence
    $command[$i] = "'$command[$i]'"; # Wrap value in single quotes
}

You can then join the array to make a single string for transmission to execute_command.

---

It's better to write that as system { $command[0] } @command to handle the case where @command has one element. This is one of the things I talk about in the "Secure Programming Techniques" chapter of Mastering Perl. (@Brian D Foy).

As a general rule, I'll accept this correction. I'm not sure it is crucial in this instance, though, where the command name is provided by the program and it is only the arguments that are possibly provided the user. The command name 'trash-put' is safe from shell expansions (IFS is reset to default by the shell when it starts, so that avenue of attack is not available).

This issue is discussed in the 'perldoc -f exec' man page:

If you don't really want to execute the first argument, but want to lie to the program you are executing about its own name, you can specify the program you actually want to run as an "indirect object" (without a comma) in front of the LIST. (This always forces interpretation of the LIST as a multivalued list, even if there is only a single scalar in the list.)

Example:

   $shell = '/bin/csh';
   exec $shell '-sh'; # pretend it's a login shell

or, more directly,

   exec {'/bin/csh'} '-sh'; # pretend it's a login shell

When the arguments get executed via the system shell, results are subject to its quirks and capabilities. See "STRING" in perlop for details.

Using an indirect object with exec or system is also more secure. This usage (which also works fine with system()) forces interpretation of the arguments as a multivalued list, even if the list had just one argument. That way you're safe from the shell expanding wildcards or splitting up words with whitespace in them.

   @args = ( "echo surprise" );
   exec @args;              # subject to shell escapes
                            # if @args == 1
   exec { $args[0] } @args; # safe even with one-arg list

The first version, the one without the indirect object, ran the echo program, passing it "surprise" an argument. The second version didn't; it tried to run a program named "echo surprise", didn't find it, and set $? to a non-zero value indicating failure.

---

¹ How do you get a back-tick to display in Markdown?

Answer 2

As stated in perldoc -f system:

If there is more than one argument in LIST, or if LIST is an array with more than one value, starts the program given by the first element of the list with arguments given by the rest of the list. If there is only one scalar argument, the argument is checked for shell metacharacters, and if there are any, the entire argument is passed to the system's command shell for parsing (this is "/bin/sh -c" on Unix platforms, but varies on other platforms). If there are no shell metacharacters in the argument, it is split into words and passed directly to "execvp", which is more efficient.

I like to use IPC::System::Simple's version of system(), which gives more control, such as being able to capture various exceptions and handle certain error codes as "bad" and others as "good":

use IPC::System::Simple qw(system);
system("cat *.txt");   # will die on failure