Launching an interpreted script

Question

I was just wondering if this is a good example, following good practices, of a C program for launching an interpreted script from a native binary executable. Here the interpreter is Perl and the script an implementation of something analogous to du --apparent-size, but the idea is general.

Also, is this code secure against potentially hostile users, at least if Perl is the interpreter (for instance, in a suid or sgid program)?

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
int main(int argc, char** argv)
{
 char* command[argc+3];
 command[0] = "/usr/bin/perl";
 command[1] = "-W";
 command[2] = "-T";
 command[3] = "/home/demetri/bin/awklikeperl.pl";
 for (int i = 0; i < argc; i++) {
 command[i+4] = argv[i+1];
 }
 execv(command[0], command);
 int error = errno;
 fprintf(stderr, "Failed to exec: %s\n", strerror(error));
 if (error == ENOENT) {
 return(127);
 } else {
 return(126);
 }
}

Answer 1

No it is not safe:

1. Classic off-by-one error. You reserve space for 3 additional entries with command[argc+3] but you add 4. argv[i+1] and command[i+4] will be out of bounds on the last iteration (last valid index is argv[argc-1] and command[argc+2] respectively)
2. execv expects a NULL terminated array of NULL terminated strings. So the last entry in commands needs to be NULL (otherwise how would execv know how many arguments to pass when starting the process?).

• Demi: You are correct about the off by one error. However, argv[argc] is guaranteed to be 0 by the C standard.
• ChrisWue: Ah true, it's a NULL terminated list, I forgot

Answer 2

Consider starting the child process in a sandbox. /usr/bin/perl is likely safe from a malicious user since /usr/bin is typically locked down by the root user(s). But /home/demetri/bin/awklikeperl.pl could be replaced if the owner is not careful with permissions. Putting the process in a sandbox will not only protect the rest of your system from attack, but help you think about what files will be made available.

Obviously, the content of the script should be reviewed for vulnerabilities as well. Take a look at perlsec for help on that front.

3 is a bit of a magic number in the C program. As you've already discovered, it's easy to forget to change that number if you add or subtract parameters from the command array. If you define that in one place (say #DEFINE STARTING_ARGS 3) you'll only need to change the value in one place.

Speaking of which, I considered suggesting adding a -- parameter to prevent any shenanigans with adding extra switches. But once you pass the script name to perl, it passes all other arguments onto the script. So I think it's not possible to use -e to execute an arbitrary command.

• rolfl: Where would you put the -- if you suggested it? The only place it would make sense is immediately before the script-to-run (i.e. before command[3]) - And, since the path/script name itself is "clean", it won't be interpreted as a perl argument. Having said that, I would actually recommend that you actually do suggest that ;-) - that way a later change to the script that's run - even a dynamic script, possibly, will still work - including if someone were to make their input file - or the traditional "use standard in" cat myperl.pl | perl -W -T -- -
• h.j.k.: If we're relying on chroot to sandbox perl, it looks like it's not too hard to break out of it too...
• Caridorc: You say that the OP should use #DEFINE STARTING_ARGS 3 but #DEFINE does not enforce type safety and is a nightmare when debugging, I suggest const int STARTING_ARGS = 3 at the start of main to overcome these issues.

Answer 3

Your fprintf() call can be written more simply using perror():

perror("Failed to exec")