Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 2.4.5

Request #17411 Please include this Debian patch in this upstream source
Submitted: 2010-05-19 00:00 UTC
From: zigo Assigned:
Status: Open Package: Net_Ping (version 2.4.5)
PHP Version: 5.3.2 OS: Debian
Roadmaps: (Not assigned)    
Subscription  


 [2010-05-19 00:00 UTC] zigo (Thomas Goirand)
Description: ------------ Hi, Net_Ping still uses escapeshellcmd when it should in fact use escapeshellarg. Please fix. I have included the patch that I am using for Debian, as I maintain this package there.

Comments

 [2010-05-19 00:01 UTC] zigo (Thomas Goirand)
 [2010-05-21 05:53 UTC] doconnor (Daniel O'Connor)
Thanks for the patch, Thomas! Any chance of a quick test script to demonstrate the security flaw (and that your fix solves it)?
 [2010-05-21 06:04 UTC] doconnor (Daniel O'Connor)
At the moment, I've only implemented the escapeshellarg command; as my regexp-fu is weak. I'm a touch hesitant to add the overstrict validation; because I don't know about internationalization of domain names or other bits and pieces like that well enough to make that change.
 [2010-05-21 06:43 UTC] doconnor (Daniel O'Connor)
-Status: Open +Status: Feedback
Thank you for taking the time to report a problem with the package. This problem may have been already fixed by a previous change that is in the SVN of the package. Please checking out the SVN repository of this package and upgrade svn checkout svn.php.net/repository/pear/packages/Net_Ping/trunk pear upgrade package2.xml or pear upgrade package.xml If you are able to reproduce the bug with the latest SVN, please change the status back to "Open". Again, thank you for your continued support of PEAR.
 [2010-05-22 22:17 UTC] zigo (Thomas Goirand)
Hi, From the PHP doc: " escapeshellcmd() escapes any characters in a string that might be used to trick a shell command into executing arbitrary commands. This function should be used to make sure that any data coming from user input is escaped before this data is passed to the exec() or system() functions, or to the backtick operator. Following characters are preceded by a backslash: #&;`|*?~<>^()[]{}$\, \x0A and \xFF. ' and " are escaped only if they are not paired. In Windows, all these characters plus % are replaced by a space instead." and also: "escapeshellarg() adds single quotes around a string and quotes/escapes any existing single quotes allowing you to pass a string directly to a shell function and having it be treated as a single safe argument. This function should be used to escape individual arguments to shell functions coming from user input. The shell functions include exec(), system() and the backtick operator." Using escapeshellcmd() is wrong, because you can still give multiple arguments when only one should be there. For example, I could send "-f 8.8.8.8" instead of just an IP address, which would do a ping flood attack which is clearly not what is intended. To avoid ANY possibility of ANY attack, we got to make sure that what is given as a parameter is either: - An IP address - A valid FQDN This is what my preg_match call does. Considering the time it takes to do 4 pings, adding a regular expression check isn't an issue. Also, having one more check can't hurt, really! Please add my patch and release a 2.4.6. Thomas
 [2012-01-27 19:16 UTC] doconnor (Daniel O'Connor)
-Status: Feedback +Status: Open