Hi, this is an excellent project. And since it's written in PHP, you don't even need to switch your thinking between the adaption of this scanner and the modification of its target. And on top it's impressively simple. Despite me having no experience with more complex parsers.
And there we are: The original source (rips-0.54.zip) showed a massive amount of false positives on my webspace. I would like to decrease them to a tolerable amount.
For example: Because i create logs out of a PHP-autoprepend, every single PHP file gets a false positive for "Userinput reaches sensitive sink. (Blind exploitation)". Despite this operation being completely innocent and unexploitable whatsoever.
Another Example: A read access to an $_ENV-Variable in the same autoprepend caused another false positive for every single PHP-file.
Since the organization if this scanner is so beautifully simple, i realized how simple it is to suppress the second type of false positive, where the culprit can simply be commented out. It's even simple to add a switch in d the GUI to exclude those candidates dynamically by clicking a checkbox.
With the first type, it's not that simple. I could introduce fake functions into my code and - simply (nice!) - add a - fake - entry to "config/securing.php", but this doesn't look right!
Wouldn't it be much better to have a possibility to let EXCEPTIONS be defined to the rules?
So that you could say: "file_put_contents with user input is bad, but not when in the file 'xyz.php' or not when with the argument 'that bloody logfile in that appendonly logdir'"…
I have taken a look at the "tokenizer.php" and "scanner.php", but without a good documentation, it seems a bit complicated for me to get an anchor for such exceptions. I would be happy to find someone with the same wish and would like to exchange ideas on the matter.
Another matter are false positives because of ignored variable initializations. One example:
199: for($i = *; $i < 20; $i++) // einbruch.php
101: for($i = *($_crackerips); $i--; ) // einbruch.php
100: $_crackerips = explode(PHP_EOL, $_crackerips); // einbruch.php
261: $_crackerips = file_get_contents($crackerips); // serverconfig.php
file_get_contents in the context of a logfile again as the source of the trouble, but…
But it wouldn't be necessary for the message to propagate through to line 199 of "einbruch.php", since in that line, the variable $i - which is considered tainted by the result of a countdown at line 101 - is initialized to zero in the code:
for($i = 0; $i < 20; $i++) if (isset($_SERVER['r'.$i]))
$regeln .= $i.', ';
…But for some reason, the scanner ignores that re-initialization.
Again, it would be simple to circumvent that specific case (never reusing any variable name), but he scanner should better honor such reuse of variable names. I have never considered naming a simple counter variable other than $i except it would be intended for being nested.
This case is repeated in many instances just at the top level of my webspace (where no big deals are going on with php).
Just another case: Sometimes, with "verbosity level" at 1 (where only vulnerabilities with "user tainted" input should show up), false positives are generated without any user input - on hard coded input for possibly vulnerable functions. You get only a yellow dot somewhere in the dependencies.
Considering EXCEPTIONS again… I stumble upon the following cases of exceptional circumstances:
* location of specific potential vulnerablilities in specific php files, possibly in conjunction with some of the other exception conditions
* specific parameter names or values in potentially vulnerable function calls (f.e. file output to logfiles/cachefiles; eval with hard codes string values)
* specific conditions for conditional blocks containing potentially vulnerable function calls (f.e. debug output restricted to lcalhost request source)
It's not necessary to code the logic to automatically handle those cases, but it would be very helpful to implement a sufficient exception condition mechanism, so that you can manually define exceptions after having inspected the marked code blocks and having assured that they contain no exploitable code.
Thus combining a sufficiently sensitive default behavior with a sufficiently robust individually definable exception scheme.
The next big class of false positives are inputs that get mapped to (hard coded) whitelists. As long as the input is only checked against the whitelist, but kept in its variable and used after clearing, it is still marked as "Cross-Site Scripting".
In some cases this adds up to dozens of false positives per file.
Did i already mention cases where a complete set of user input is run through a "securing function"? While falsely mistaking the residuals of counter variables as initialized and ignoring re-initializations of those, the scanner does not realize the fact that foreach really IS "for each" of the data. Again: It may be difficult to program the logic in a way that prevents introduction of false negatives, but it would be relatively easy to define some regex as user to exclude those cases from false positives.
Fazit for now: This Scanner helped me to secure a handful of vulnerabilities, mainly XSS.
It would be nice if we could get him to a somewhat lower rate of false positives (which was at about 2000 entries at the first run on about 500 files, partially up to 20 false positives per file). Thanks to its configurability, this rate could be significantly lowered by the application of user defined entries in the config files, but still remains somewhat very high. But despite that rate, the scanner proved its usefulness.
Thanks to the creator!
I'm still interested in collaboration towards configurable exceptions…