Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escapeArgs option is not working properly #44

Closed
Kirill89 opened this issue Dec 18, 2019 · 2 comments
Closed

escapeArgs option is not working properly #44

Kirill89 opened this issue Dec 18, 2019 · 2 comments

Comments

@Kirill89
Copy link
Contributor

I expect that addArg() escapes all arguments preventing possibility of command injection from untrusted sources.

Because in the README.md I see:

  • Handle argument escaping

  • $escapeArgs: Whether to escape any argument passed through addArg(). Default is true.

But it is not actually happens.

PoC:

<?php
require_once 'vendor/autoload.php';

use mikehaertl\shellcommand\Command;

$command = new Command(array(
    'command' => 'curl',
    'escapeArgs' => true,
));
// In this example "escapeArgs" is set to "true", but escaping is not happens.
$command->addArg('http://example.com --wrong-argument || echo "RCE 1"');
$command->execute();
echo $command->getOutput(); // RCE 1

$command = new Command(array(
    'command' => 'curl http://example.com',
    'escapeArgs' => true,
));
$command->addArg('http://example.com');
// In this example, the second argument will be escaped properly, but the first one - not.
$command->addArg('--header foo --wrong-argument || echo "RCE 2" ||', 'bar');
$command->execute();
echo $command->getOutput(); // RCE 2

Disclaimer

This thread was initially started as a private email conversation. @mikehaertl asked me to open an issue here.

@Kirill89 Kirill89 changed the title escapeArgs option is not working properly escapeArgs option is not working properly Dec 18, 2019
@mikehaertl
Copy link
Owner

Thanks. If you have an idea for a fix, please let me know or provide a MR. Some examples of the expected output would also be great. I could add it to the tests so we can verify that the problem is fixed later.

Kirill89 added a commit to Kirill89/php-shellcommand that referenced this issue Dec 19, 2019
Kirill89 added a commit to Kirill89/php-shellcommand that referenced this issue Dec 19, 2019
@mikehaertl
Copy link
Owner

Released 1.6.1 containing this fix. Thanks for bringing this up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants