This exec() is secure?

Question

I have this code :

$cmd = escapeshellcmd($_GET['command']);
$cmd2 = escapeshellcmd($_GET['command2']);
exec("program" . $cmd . $cmd2);

And I want know if is secure to allow any person to use it with an html form ?

I dont know enough about this, but it seems pretty unsafe. How about I say `&& touch myprog.sh && echo "my program code" >> myprog.sh`? — msfoster, Apr 21 '14 at 21:37
You are executing `"program$cmd$cmd2"`, not `"program $cmd $cmd2"`. And if you have to ask then it is probably not secure. — Sverri M. Olsen, Apr 21 '14 at 21:44

score 3 · Accepted Answer · edited May 23 '17 at 12:11

3

No, don't let using escapeshellcmd or escapeshellarg give you a false sense of security. Passing values directly to the commandline even escaped without any form of validation is asking for trouble.

If you are adamant I'd advise reading the following discussion:

What's the difference between escapeshellarg and escapeshellcmd?

edited May 23 '17 at 12:11

Community

1
1

answered Apr 21 '14 at 21:47

Daniël W. Crompton

3,448
25
26

2

+1 for a strict NO. OP, you should _always_ whitelist arguments explicitly. – Benjamin Gruenbaum Apr 21 '14 at 21:49
Ok, thanks. Do you have any idea to secure this part of code ? – esffsefesffsesef Apr 21 '14 at 21:49
2

@skyice Benjamin just told you: white list the allowed arguments – nl-x Apr 21 '14 at 21:51
@skyice you should read the link I posted for advise on doing it too – Daniël W. Crompton Apr 21 '14 at 21:52

score 0 · Answer 2 · answered Apr 21 '14 at 21:53

0

You need to whitelist allowed commands.

$cmd = escapeshellcmd($_GET['command']);
$cmd2 = escapeshellcmd($_GET['command2']);
$allowed_cmds = array('ls', 'foo', ...);

if(in_array($cmd, $allowed_cmds)) {
    if(in_array($cmd2, $allowed_cmds)) {
        exec("program" . $cmd . $cmd2);
    }
}

Personally though, you are doing something wrong. I've written socket systems, websites, database abstractions, language processing libraries, email servers, regex parsers, and a whole slue of other things.

I've never needed the nasty, slow exec.

answered Apr 21 '14 at 21:53

Xeoncross

55,620
80
262
364

1

You probably meant to use `escapeshellarg` and you need to add spaces to the exec. It's clear you haven't used it very often. :) – Daniël W. Crompton Apr 21 '14 at 21:56
1

@DaniëlW.Crompton, that's what I get for cheating. I just copied the OP's code and throw in the conditional. I learned my lesson. – Xeoncross Apr 22 '14 at 03:10

This exec() is secure?

2 Answers2