I recently noticed an article that Classic ASP, ColdFusion, and PHP apps tend to have the most security bugs. The article mentions 'static analysis' and 'scanned applications.' It sounds to me like Veracode (who published the underlying report) might have some scheme to automate a security scan of a PHP application. Is there any such tool available to us writers of PHP apps? Seems like a handy thing to have -- although such a scanner sounds like a really difficult thing to construct. How can one possibly trace all the pathways from user input into the app?
is there a tool to scan one's PHP code for security problems?
You do it function by function (you do have reasonably small functions, and you do do coverage tests, right?)
Just browsing for news today I ran across https://insight.sensiolabs.com/ that you might want to look at. Also look up "cyclomatic complexity", which is a measure of how many code paths there are through a given routine.
If you want to mess around with VLD (by the author of XDebug) that does codepath analysis.
Your eyes. 
Weedpacket;11052571 wrote:You do it function by function (you do have reasonably small functions, and you do do coverage tests, right?)
I will admit (with substantial embarassment) that I have not been conscientious about establishing automated testing for my code. Suggestions for learning resources would be much appreciated.
The reason I asked this particular question is because of a very old site that I wrote some 12 years ago. No testing then. Poorly organized procedural code. Nearly 1000 files. I'm working on its replacement but must keep the old site limping along.
Thanks for the other suggestions.
Bonesnap wrote:Your eyes.
:glare: Look at the cheek on you!
While I would like to collect the paycheck for examining every line of code in the site (~1000 php files), I think I would off myself.
sneakyimp;11052609 wrote:While I would like to collect the paycheck for examining every line of code in the site (~1000 php files), I think I would off myself.
And, of course, at least at this shop, the check you received would be for no greater amount than the one you already received for writing it in the first place.
And both of them don't have enough zeroes on the end 
sneakyimp;11052563 wrote:is there a tool to scan one's PHP code for security problems
PHPQATools.org wrote:PHP Mess Detector
ROFLMAO!
hey MD and CPD are the best ever when you pick up an existing project and need to do work in it.
Derokorian;11052639 wrote:hey MD and CPD are the best ever when you pick up an existing project and need to do work in it.
I was just laughing at the name!
I pulled down a copy ... so far, SFTP'd to the local VM, next had to learn about PHARs and figure out how to extract it, read some docs, then try and figure out how in Heaven's name to run it ... (stuck there now), and all that with bugs breathing down my neck, a complex issue with my brother regarding Mother's estate, my wife calling because her boss is having email trouble ... #geeklife #monday_was_yesterday
And only 78 git operations on one project today so far (not counting the automated pulls from the production boxen) ...
An honest question ... does PHPMD only detect messes in Symfony projects and such? What about custom code?
dalecosp;11052641 wrote:I pulled down a copy ... so far, SFTP'd to the local VM, next had to learn about PHARs and figure out how to extract it, read some docs, then try and figure out how in Heaven's name to run it ... (stuck there now)
Generally, all I do with phars is download, give permission to execute and then run it. Something like (from phpunit docs):
wget https://phar.phpunit.de/phpunit.phar
chmod +x phpunit.phar
sudo mv phpunit.phar /usr/local/bin/phpunit
phpunit --version
dalecosp;11052641 wrote:An honest question ... does PHPMD only detect messes in Symfony projects and such? What about custom code?
yeah the first argument is the path to source, I've never had any problem running on my code before.
Derokorian;11052643 wrote:Generally, all I do with phars is download, give permission to execute and then run it. Something like
Thankies much!!!
Hmm, it seems threaded_fetch() has an NPath complexity > 4000 :p
Have you ever run it against a threaded application?
I'm not sure I get your meaning dale - I guess it depends on how are you threading, since the language itself is single-threaded...
FWIW, php depend and php mess detector don't work for me, because I'm using return types (new in php7) HAHA....
PS. For the original topic, I realized now that the qa tools don't necessarily help with security analysis. So I should share, that I've used a bunch of the tools on this page: https://www.owasp.org/index.php/Appendix_A:_Testing_Tools to help find problems in projects.
I've got a threaded_fetch function; it takes an array of URI's and a folder path to write the files into. It can run up to 24 simultaneous threads using [man]pcntl_fork/man.
The guts of the thing look like this:
$current_pid=posix_getpid(); // Get the PID of the curently running process
if ( $current_pid == $pids[0] ) { /* Task for child 1 */
if (count($url_chunks[0])) {
systemFetchSequential( $url_chunks[0],"$outpath" );
}
exit();
} elseif ( $current_pid == $pids[1] ) { /* Task for child 2 */
if (count($url_chunks[1])) {
systemFetchSequential( $url_chunks[1],"$outpath" );
}
exit();
} elseif ( $current_pid == $pids[2] ) { /* Task for child 3 */
if (count($url_chunks[2])) {
systemFetchSequential( $url_chunks[2],"$outpath" );
}
exit();
} elseif ( $current_pid == $pids[3] ) { /* Task for child 4 */... etc.
24 times. I'm not at all certain it can be done any other way. I guess I need to play with it and see. (Of course, as if I have time to do that ...)
um hold on... are you telling me you use pcntl_fork to do http requests? If so, you should definitely look into [man]curl_multi_exec[/man]. It allows you to perform multiple requests in asynchronously, including the ability to add on requests while continuing to process the existing requests. Here's a probably overly verbose example from actual code, I don't feel like chopping it down for you 
<?php
/**
* @param $sUrl
* @param array|null $sPostData
* @param array|null $aHeaders
* @return resource
*/
function getCurl($sUrl, $sPostData = null, $aHeaders = null)
{
$oHandle = curl_init($sUrl);
curl_setopt($oHandle, CURLOPT_RETURNTRANSFER, true);
curl_setopt($oHandle, CURLOPT_HEADER, true);
curl_setopt($oHandle, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($oHandle, CURLOPT_SSL_VERIFYPEER, false);
curl_setopt($oHandle, CURLOPT_SSL_VERIFYHOST, false);
curl_setopt($oHandle, CURLOPT_TIMEOUT, 60);
curl_setopt($oHandle, CURLOPT_CONNECTTIMEOUT, 15);
if (!empty($sPostData)) {
$aHeaders[] = 'Content-Type: application/json';
curl_setopt($oHandle, CURLOPT_POST, true);
curl_setopt($oHandle, CURLOPT_POSTFIELDS, $sPostData);
}
if (!empty($aHeaders)) {
curl_setopt($oHandle, CURLOPT_HTTPHEADER, $aHeaders);
}
return $oHandle;
}
/**
* @param resource $hCurlMulti Resource returned by curl_multi_init
* @param $aCurls
* @param $aTestCases
*/
function runCurlMulti($hCurlMulti, &$aCurls, &$aTestCases)
{
list($mrc, $active) = curlMultiExec($hCurlMulti);
while ($active && $mrc == CURLM_OK) {
$iReady = curl_multi_select($hCurlMulti);
if ($iReady != 0) {
do {
$mrc = curl_multi_exec($hCurlMulti, $active);
} while ($mrc == CURLM_CALL_MULTI_PERFORM);
}
if ($iReady > 0) {
// A request has finished
$aInfo = curl_multi_info_read($hCurlMulti);
while ($aInfo !== false) {
$sTestId = array_search($aInfo['handle'], $aCurls);
$hCurl = $aCurls[$sTestId];
list($sTestId, $sType, $sHandleId) = explode('-', $sTestId, 3);
if ($aInfo['result'] === CURLE_OK) {
$aTestCases[$sTestId]['response'][$sType][$sHandleId]['stats'] = curl_getinfo($hCurl);
$iHeaderSize = curl_getinfo($hCurl, CURLINFO_HEADER_SIZE);
$sResponse = curl_multi_getcontent($hCurl);
$aTestCases[$sTestId]['response'][$sType][$sHandleId]['headers'] = trim(substr($sResponse, 0, $iHeaderSize));
$aResources = parseResponseForResources(
trim(substr($sResponse, $iHeaderSize)),
sprintf(
"%s://%s",
parse_url($aTestCases[$sTestId]['url'], PHP_URL_SCHEME),
parse_url($aTestCases[$sTestId]['url'], PHP_URL_HOST)
)
);
foreach ($aResources as $sResource) {
$aCurls["$sTestId-$sType-$sHandleId-$sResource"] = getCurl($sResource);
curl_multi_add_handle($hCurlMulti, $aCurls["$sTestId-$sType-$sHandleId-$sResource"]);
}
if (!empty($aResources)) {
list($mrc, $active) = curlMultiExec($hCurlMulti);
}
} else {
// Request failed, handle it
$aTestCases[$sTestId]['response'][$sType][$sHandleId]['error'] = array(
curl_errno($hCurl),
curl_error($hCurl)
);
}
// remove this handle since we've recorded its stats
curl_multi_remove_handle($hCurlMulti, $hCurl);
// read more info, in case multiple handles finished
$aInfo = curl_multi_info_read($hCurlMulti);
}
}
}
}
/**
* @param resource $hCurlMulti Resource returned by curl_multi_init
* @returns array
*/
function curlMultiExec($hCurlMulti)
{
do {
$mrc = curl_multi_exec($hCurlMulti, $active);
} while ($mrc == CURLM_CALL_MULTI_PERFORM);
return array($mrc, $active);
}
$hCurlMulti = curl_multi_init();
$aTestCases = json_decode(file_get_contents($argv[1])) or die('Invalid test case file specified');
foreach ($aTestCases as $k => $aTestCase) {
$aCurls["$k-page-get"] = getCurl($aTestCase['url']);
curl_multi_add_handle($hCurlMulti, $aCurls["$k-page-get"]);
}
runCurlMulti($hCurlMulti, $aCurls, $aTestCases);
curl_multi_close($hCurlMulti);
And if you need it to go directly to a file, you can use:
$hFP = fopen('/path/to/some/file', 'w');
curl_setopt($hCurl, CURLOPT_FILE, $hFP);
Additionally, you could reduce the cycolmatic complexity by using an [man]array_search[/man], instead of a bunch of manual if statements...
$current_pid=posix_getpid(); // Get the PID of the curently running process
if (($k = array_search($current_$pid, $pids)) !== false &&
count($url_chunks[$k])) {
systemFetchSequential( $url_chunks[$k],"$outpath" );
}
And while I'm commenting on code... why "$outpath" instead of just $outpath? There seems to be no reason to surround this in quotes...
Derokorian wrote:Here's a probably overly verbose example from actual code, I don't feel like chopping it down for you
Well, I didn't chop mine up for you either ... fair's fair, right? 
We also have a function named curlMultiRequest() ... I assume system performance is your objection? Hardware is typically cheaper than programmer time, right? In terms of fixing this, I'm not sure I feel like looking through all 60+ front end modules to see which use cURL vs. fetch today (or next week, either 
) ... but I could probably adjust threaded_fetch() to use [man]array_search[/man] the next time I play around with a threaded module and see if it breaks anything (we at least have a sandbox for this app, which is better than several of our projects).
As for "$outpath", I dunno; I just posted the code as it stands. We're a tad pragmatic here; it didn't produce a bug, so it didn't get noticed. As I have primary git access I'll commit a change on that soon. Thanks for your free QA testinHHHHHHHHHH^H^H^H^H feedback!
Management/Mods: How about a BBCode for <strike></strike>? Please?
I have no comment on system performance, I use curl_multi because well someone WAY smarter than me has done all the threading work, and I don't have to try to make a language designed to be single threaded work with multiple threads I literally do it because it puts off the hardest part of something like this on someone else's well tested and highly used code, as opposed to something I write which is - well not guaranteed to be good at all.
dalecosp;11052723 wrote:As for "$outpath", I dunno; I just posted the code as it stands. We're a tad pragmatic here; it didn't produce a bug, so it didn't get noticed. As I have primary git access I'll commit a change on that soon.
Of course it doesn't produce an error, either way you end up with a string whose value is equal to outpath in the function. Just seems unnecessary to make the engine interpolate a double quoted string and replace the variable into that string, just to have the same end result as only passing the variable.
dalecosp;11052723 wrote:Thanks for your free QA testinHHHHHHHHHH^H^H^H^H feedback!
No problem, but why are you raising testing to the power of H to the power of H to the power of....
dalecosp;11052631 wrote:And, of course, at least at this shop, the check you received would be for no greater amount than the one you already received for writing it in the first place.
And both of them don't have enough zeroes on the end
You may want to check your salary against the averages in your area:
http://www.myplan.com/careers/software-developers-applications/salary-by-state-15-1132.00.html
Derokorian;11052669 wrote:FWIW, php depend and php mess detector don't work for me, because I'm using return types (new in php7) HAHA....
Can you clarify the issue with return types? I'm guessing there's some new-fangled syntax to declare return types that confuses the code parser in these tools.
Derokorian;11052669 wrote:PS. For the original topic, I realized now that the qa tools don't necessarily help with security analysis. So I should share, that I've used a bunch of the tools on this page: https://www.owasp.org/index.php/Appendix_A:_Testing_Tools to help find problems in projects.
So I'm wondering how we feel about just looking at some massive list of tools like this, downloading them, and then running them. Were I a hacker of any great skill, I would obviously construct these tools to report vulnerabilities to my nefarious criminal organization. I'd also write the source code scanners to send all the certificates, passwords, and payment credentials to my botnet for immediate exploitation.