|Resolved||csteipp||T110617 Goal: Implement static code analysis for security|
|Resolved||csteipp||T121046 Automatically submit weekly core deployment branch (+skins, +vendor) to Veracode|
During Q2 we again investigated Veracode, Checkmarx and Pfff.
- Pfff - taint-tracking is still closed-source at facebook. Variable analysis is too noisy to be useful. As a result, we cannot use pfff for static security analysis at this time.
- The semantic patching may be useful as part of a preparation pipeline for submitting to other tools.
- We will continue to evaluate pfff in the future and track their progress. @dpatrick may be able to implement our own version of their taint tracking.
- Checkmarx - As a tool, Checkmarx gives the most flexibility (allowing us to write our own rules), and most useful information about security flaws (shows full path that triggered security rule through code). Running on our own servers gives us good visibility into how the tool is performing for us. Those debugging logs, however, showed us that the tool was failing to parse significant portions of the MediaWiki core + tarball extensions, due to unexpected syntax. The tool was not able to parse closures as function parameters, assigning object properties using list(), and some other similar issues. These, along with the proposal to move mediawiki to use PHP 5.5 syntax which seems likely to be a challenge for Checkmarx to parse given the types of issues we've seen in 5.3 syntax, precludes us from using the tool.
- Veracode - Trial scan seemed to give legitimate results for code in core, but it was unable to trace taint issues through extensions. During the evaluation, we found a potential false-negative, and their engineers indicated they would be adding detection for that pattern in a future release. In addition to being responsive to improvement suggestions, Veracode was willing to give us a significant price discount, so we will be using them to scan deployment branches of core each week.
Core + PdfHandler are being submitted to Veracode after each branch cut. This should alert the security team to potential issues within core, and hopefully improvements to the Veracode platform will allow the tool to trace data flows within PdfHandler in the near future. The Security Team will look for ways to automate triage and reporting issues into Phabricator in the future.
Just FYI, we are no longer using veracode. The reports had too many false positives, and was not able to trace code execution well enough to catch many things with extensions. The result was nobody was reading the weekly report, so we stopped around nov 2016.
We are instead now moving to our home-grown phan-taint-check static analysis tool.