Do I need to sanitize $_SERVER['argv']? How?

Question

I tested my PHP application for vulnerabilities, with Checkmarx. I got File Manipulation error in $_SERVER['argv'].

$argv = $_SERVER['argv'];
if (count($argv) < 3) {
    echo "Usage: ".htmlentities($argv[0])." OldName NewName\n";
    exit(1);
}

Do I need to sanitize $_SERVER['argv']? How?

Nobody knows what _file manipulation error_ is, it's not a PHP error. — AbraCadaver, Aug 31 '20 at 14:48
it's an issue I found in my application's SAST report . all the arguments passed from command line, are stored in $_SERVER['argv'] array . So whenever someone is giving input in cmd and runs the php script . it stored into $_SERVER['argv'] . Now I think I have to sanitize the input data . It's complete guess for me . — Maximious, Aug 31 '20 at 14:57
You only need to sanitize values before you use them for something. Then you need to sanitize it for that specific use case. In your example, if that script is called through CLI, then `htmlentities()` will be pretty useless since the command line won't parse the output as HTML (which is what that function sanitizes) either way. — M. Eriksson, Aug 31 '20 at 15:14
`$argv[0]` refers to the script that's running, so I would imagine checkmarx believes this is a vector for the script to overwrite itself. It may be just alerting on any reference to `$argv[0]`. The only vulnerability I can see here is a possible path disclosure, which you can address by stripping the directory off via `basename($argv[0])`. — Alex Howansky, Aug 31 '20 at 15:52
This doesn't make any sense to me. If you're executing this script from the command line, then a) $argv is already set for you, you don't need to pull it from anywhere and b) when executed from CLI $argv[0] is always the name of the executing file itself; unless you named that something malicious somehow, you shouldn't need to sanitize it. If instead this is serving a web request, there's better ways to retrieve GET and POST variables, namely `$_GET` and `$_POST` supervars. But maybe I'm missing something here. — parttimeturtle, Aug 31 '20 at 15:54
@parttimeturtle you are right . This doesn't make sense to me also but checkmarx marcked it as vulnerability . if malicious file name is given while executed from CLI then it can be flagged as vulnerability — Maximious, Aug 31 '20 at 16:06
Can you share the *Checkmarx* report message? Maybe you use the `$argv` after the `if`? We need to see the *Checkmarx* message to know the flow from the *source* to the *sink* — baruchiro, Sep 01 '20 at 06:10

score 0 · Answer 1 · answered Sep 01 '20 at 06:35

0

Checkmarx is looking for sanitizers so try this sanitization approach using stripslashes, basename or realpath function:

$argv = realpath($_SERVER['argv']);
if (count($argv) < 3) {
    echo "Usage: ".htmlentities($argv[0])." OldName NewName\n";
    exit(1);
}

answered Sep 01 '20 at 06:35

securecodeninja

2,497
3
16
22

Thanks for your reply . I will try this with realpath – Maximious Sep 01 '20 at 06:37
one more thing how do I sanitize $argv[1] , $argv[2] ... I did added filter_var($argv[1], FILTER_SANITIZE_STRING) but still checkmarx shows it as vulnerability – Maximious Sep 01 '20 at 06:56
is Checkmarx showing the same vulnerability type (File Manipulation)? filter_var with FILTER_SANITIZE_STRING will protect you from XSS – securecodeninja Sep 01 '20 at 19:09
can you add the code from your comment here into the question you have above so we can have a clear picture of the flow? – securecodeninja Sep 02 '20 at 03:26

Do I need to sanitize $_SERVER['argv']? How?

1 Answers1