With Magento Coding Standard and Code Sniffer rules update to v3 becoming effective July 11th, insecure functions like exec(), shell_exec(), system() etc. will be disallowed. That means any extension using one of these functions will be denied to be listed in Magento Marketplace. Since these functions can be a huge security issue, at least when used in a bad way, it is certainly a good decision by Magento to take this step towards improving Magento Security.
But what does this mean for Magento developers, what alternatives to these insecure functions are available to implement certain features that require system commands to be executed on server level?
And, first of all, how good are these alternatives from a security perspective?
What needs to be considered to ensure functions for executing system commands are built in a secure not exploitable way?
Why are PHP exec(), shell_exec(), system() etc. insecure and should be avoided, not only in Magento?
Let’s take a look at the PHP documentation for exec():
exec() executes the given command.
Yes, short and simple, that is any command available from your CLI on your server. A quite massive tool providing the whole glory of CLI power to your PHP application!
Basically when using exec() e. g. to execute a fix command as required by your application and there is no way for a hacker to tamper with this command, there would be nothing wrong with exec().
But as soon as exec() is used to run some command depending on user input or having some way to manipulate the command to be executed, it’s like a dream come true for any hacker. Opening the door widely to compromise your application and server. So better be careful when making use of this function in your code or Magento extension and keep in mind:
With great power comes great responsibility.
Think ahead, even if your implementation seems to be secure at the time of developing, the security nightmare might be created only later when your function gets changed or extended by someone not aware or missing the risks coming with exec().
As a rule of thumb, never assume your implementation using exec() is secure. With security in mind while developing, always look for alternatives that provide means of mitigation for possible attacks by avoiding or limiting the power and risks coming with exec().
What Secure Alternatives Are Available for PHP exec(), shell_exec(), system()?
Checking the PHP documentation for exec() already gives us some hint:
Warning: When allowing user-supplied data to be passed to this function, use escapeshellarg() or escapeshellcmd() to ensure that users cannot trick the system into executing arbitrary commands.
Oh, that’s great, just replacing the critical function with one of these alternatives and we are good and secure with arbitrary commands being prevented from being executed?
No, unfortunately, it’s not that easy…
Of course, finding a solution that meets your requirement without executing system/server level commands by your PHP application at all in the first place would be the best idea from a security point of view. But that might not be wanted / feasible in every case. So what options do we have for executing system commands while making sure it is done in a secure way?
More Secure Alternatives to exec(): escapeshellarg() + escapeshellcmd()
As described above none of the alternatives provides us a sufficient, not to speak of a bullet-proof, level of protection that works as a turnkey solution in every scenario against command injection attacks while allowing to execute system commands by PHP.
The recommended way is to still make use of at least one or a combination of above-mentioned functions as a replacement for exec() or other insecure functions. But never fully rely on them when it comes to mitigating the risks of executing system commands, do not assume those functions are enough for your application to be secure.
What these functions are doing differently than exec() is escaping following characters when found within the command to be executed:
&#;`|*?~<>^()[]{}$\, \x0A and \xFF. ' and " Respectively adding single quotes around strings around an argument of a command to be executed / and quotes/escaping single quotes within an argument of a command to be executed.
It’s certainly a good idea to make use of these functions instead of exec() whenever possible as a mean of improved security.
BUT:
Be aware, these two functions do not fully mitigate the risks we are talking here!
Using these functions only might still leave your application vulnerable to command injection attacks. There are still ways to work around the additional level of protection, means it is still possible that arbitrary commands can be executed even when using escapeshellarg() and escapeshellcmd().
You can find a nice list of command injection exploits and vulnerabilities including examples here.
(Please use for your educational purpose only, but not for, you know, doing some bad stuff ;-) )
Please also note that these two functions might also prevent your legit commands from being executed, e. g. in case you need some of the characters being escaped by these functions is needed by your command.
Symfony Process Component As Alternative For Insecure PHP Functions
Since Magento is using Symfony Framework, there is another alternative available that is designed to be used as a drop-in replacement for insecure PHP functions like exec(), passthru(), shell_exec(), and system(), the so-called Process Component. As you can see in the documentation, the Symfony Process Component comes with a bunch of additional features for handling commands and also does some escaping.
Unfortunately, the Process Component was not designed as full protection against command injection attacks. Also with this alternative, it might be possible that malicious/arbitrary commands can be executed by an attacker being able to tamper with the command. Escaping is only done for arguments passed as an array. Not for commands passed as a single string, which get executed as is.
The Holy Grail Of Secure Application Development: Input Validation
Instead, put on the glasses of an attacker and always ask yourself if there are any ways of tampering with the command that your application passes to the server. And if so, add your own measures of protection to close the attack vector. That is in the first place, as always in secure application development with any user input:
Consider it dirty and not trusted!
Add some input validation for any user input that is used in your command. Make it as tight as possible. Allowing and ensuring only the command needed and only the one really needed can be executed by your function.
Just for the record:
Any input validation for security reasons needs to be done server-side.
Client-side validation by JavaScript might be good to add as well. But that is for usability, not as a security measure, as it can be bypassed even by a not too skilled hacker easily.
Conclusion
The decision by Magento to disallow insecure functions like exec() in Magento Marketplace is definitely a good one.
However, just enforcing these functions to be not used is not enough to ensure a security level that is needed for an e-commerce site handling sensitive data.
But that’s not what Coding Standards aim at or are designed for. Coding Standards cannot guarantee 100% security or prevent any security vulnerability.
But at the very least, with the updated Magento Coding Standard, developers attention gets pointed to some potentially big security vulnerabilities. In the end, it’s always up to the developer to ensure an appropriate level of security, with a suitable and secure implementation depending on each individual situation.