SoftwareSecurity2014/Group 7/Code Scanning
Inhoud
Fortify
The scan by Fortify returned 228 vulnerabilities of the following severity classes:
As this was a relatively large number, we focused only on vulnerabilities of critical, high and medium severity and only those classes which were related to the security requirement. Specifically, only command injection, open redirect, path manipulation, sql injection, header manipulation, log forging and file upload were related to V5 Input Validation.
False Positives
Fortify had a number of false positives. In the table below, the first two issues are related to Fortify not detecting the input validation which has been successfully performed. Unfortunately, this seems unavoidable as Fortify cannot predict the developer's intent without additional annotation. (Erik: Still, Fortify could know about standard functions to sanatise input and be able to determine whether some input is used sat as part of an SQL query) The third issue is related to file upload which is a necessary functionality of a CMS and cannot be disabled. (Erik: Great that you go into detail on whether some warnings are false or true positives, but it would be useful to know at how many you looked at, to get an impression of how many warnings are false or true positives. Eg, are the true positives listed below the only ones you could find, or did you look at only a few warnings couple to find these?)
Warning from Fortify | Our Analysis |
Path Manipulation - Attackers can control the filesystem path argument to chmod() and chgrp() at class.t3lib_div.php line 3225, 3229, 3235 which allows them to access or modify otherwise protected files. | This is a false positive as there is a function isAllowedAbsPath ($path) which checks that the filename is sanitised (e.g. no ../..) before calling chmod. This is similar to Fortify's recommendation. |
Path Manipulation - Attackers can control the filesystem path argument to move_uploaded_file() at class.t3lib_div.php line 4489, and copy() at line 4492 which allows them to access or modify otherwise protected files. | This is a false positive as the function cleanFileName cleans the filename based on a whiltelist before move_uploaded_file() or copy() is called. This is similar to the recommendations by Fortify. |
File Upload - The function upload_copy_move() in class.t3lib_div.php calls move_uploaded_file() on line 4489. Permitting users to upload files can allow attackers to inject dangerous content or malicious code to run on the server. | Fortify recommends to either disable the file_uploads option in php.ini, or restrict the types of files to upload. Both are not possible as file upload is an important functionality of a CMS. |
True Positives
Fortify also found several actual vulnerabilities. The following vulnerabilities are a result of taking inputs from a tainted source without input sanitisation or validation.
Warning from Fortify | Our Analysis |
Path ManipulationAttackers can control the filesystem path argument to the following functions, which allows them to access or modify otherwise protected files.
a) file_get_contents() at class.t3lib_div.php line 3153 and 3164 b) copy() at class.t3lib_parsehtml_proc.php line 444 Open Redirect The file init.php passes unvalidated data to an HTTP redirect function on line 317 in class.t3lib_div.php and line 324 in class.t3lib_div.php |
The tainted information is $_SERVER['REQUEST_URI'] or $_SERVER['HTTP_HOST']. An adversary could set the server variable with malformed URLs [1] in old versions of PHP (e.g. v4.2) or on shared servers. |
SQL Injection - Line 972 of class.t3lib_db.php invokes a SQL query built using input coming from an untrusted source. This call could allow an attacker to modify the statement's meaning or to execute arbitrary SQL commands. | The function takes an argument $query and then uses mysql_query($query, …) to run it. If an attacker could control the $query variable, an SQL injection is possible. Fortify recommends to use the mysqli() function in combination with prepared statements to block these kinds of attacks. |
In the following vulnerabilities, although input sanitation has been performed, it was insufficient. For example stripslashes was used but as it was not used recursive e.g. double blackslash (\\) becomes a single blackslash (\), attackers could easily overcome it.
Warning from Fortify | Our Analysis |
Command Injection - Line 163 in view_help.php and 130 in wizard_add.php and 135 in wizard_colorpicker.php call unserialize() with a command built from untrusted data. This call can cause the program to execute malicious commands on behalf of an attacker. | This vulnerability is found in a function which reads POST and GET parameters and passes it to unserialise(). It is partly sanitized using stripslashes() but that is insufficient. Fortify recommends: “Do not allow users to have direct control over the commands executed by the program.” |
Log Forging - The program writes unvalidated user input to the log in class.t3lib_div.php on line 5987. | The program writes unvalidated user input to the log in t3lib_div.sysLog(). An attacker could take advantage of this behavior to forge log entries or inject malicious content into the log. For example, POST and GET variables are retrieved via t3lib_div._gp(), which only applies stripslashes. SERVER variables are not sanitized at all before being logged. FILES variables (file name) is not sanitized before being logged. |
Cookie Manipulation - The method setsessioncookie() includes unvalidated variable $this->id in an HTTP cookie (in the setcookie() function) in line 364 of class.t3lib_userauth.php. | The value $id is retrieved via t3lib_div._gp(), t3lib_div._get() or t3lib_userauth.getcookie(), which all apply stripslashes before returning the value. However this is still not good enough and might still lead to Cookie manipulation attacks and can lead to other HTTP Response header manipulation attacks like: cache-poisoning, cross-site scripting, cross-user defacement, page hijacking or open redirect. |
RIPS
Settings:
Different verbosity levels can be set. We ran the scan with the lowest verbosity which only considers tainted data from the user. Other levels are file tainted, db tainted, debug etc. The scan by RIPS returned 22 vulnerabilities as opposed to Fortify finding 228. It classified the vulnerabilities as follows:
All of these categories correspond to our chosen security requirement of Input Validation due to our choice of scanning preferences. Hence we checked al 22 of them:
File: /var/www/typo3_src4.5.17/typo3/index_re.php
Vulnerability type | Warning from RIPS | Our Analysis |
HTTP Response Splitting. | User input reaches sensitive sink. Triggered by: header('Location: ' . str_replace($thisPath_base, $mainPath_base, $script_name)); | True positive. The attacker might exploit this by passing malicious data through the parameters $thisPath_base, $mainPath_base, $script_name. |
CrossSite Scripting. | User input reaches sensitive sink. | Triggered by print_r function. Not exploitable. |
File: /var/www/typo3_src4.5.17/typo3/mod.php
Vulnerability type | Warning from RIPS | Our Analysis |
CrossSite Scripting. | User input returned by function _get() reaches sensitive sink. The line triggering this: $temp_M = (string)t3lib_div::_get ('M'); | True positive. The parameter M should be validated or the attacker can send malicious data through the get parameter. |
File: /var/www/typo3_src4.5.17/typo3/install/index.php
File manipulation and Disclosure attack are triggered by calling a common file /typo3conf/ENABLE_INSTALL_TOOL through $enableInstallToolFile = $PATH_site . . The "ENABLE_INSTALL_TOOL" file can be created by putting an empty file into the directory "typo3conf". You usually need write access to this directory on a server level (for example via SSH, SFTP, etc.) or you can create this file as a backend user with administrator privileges. You should delete this file as soon as you do not need to access the Install Tool any more. It should also be mentioned that TYPO3 deletes the "ENABLE_INSTALL_TOOL" file automatically if you logout of the Install Tool or if the file is older than 60 minutes (expiry time). Both features can be deactivated if the content of this file is "KEEP_FILE", which is understandably not recommended (Unless the user deactivated the features).
Vulnerability type | Warning from RIPS | Our Analysis |
File Manipulation | User input reaches sensitive sink. (Blind exploitation) the function triggering this: unlink and touch | Might be exploited because $_SERVER is being called |
File Disclosure | User input reaches sensitive sink the function : file_get_contents | Might be exploited because $_SERVER is being called |
File Inclusion | User input reaches sensitive sink | The function triggering this: require_once. True positive. require_once might be by the attacker for local file inclusion. |
File: /var/www/type3_src-4.5.17/typo3/sysext/openid/lib/php-openid/Auth/OpenID/Server.php
Vulnerability type | Warning from RIPS | Our Analysis |
Code Execution | Line 1575: Code Execution (User input reaches sensitive sink.) | This decode function is in a OpenID library. The function call_user_func_array calls any function that is passed, which might be insecure. Part of the function appears to come from user POST arguments. Valid inputs are whitelisted by the getArg function: this is a false positive. |
File: /var/www/type3_src-4.5.17/typo3/sysext/adodb/adodb/session/adodb-session-clob.php This file only calls require_once(‘adodb-session.php’), which calls require_once(‘../adodb.inc.php’), which is the culprit.
Vulnerability type | Warning from RIPS | Our Analysis |
Cross-Site Scripting | Line 486: Userinput reaches sensitive sink | Echo of an error message, which possibly contains user input that is not sanitized. If this error is printed to some website, this could lead to xss attacks. |
Cross-Site Scripting | Line 487: Userinput reaches sensitive sink | Echo of an error message. The php-function strip_tags is called before outputting the message, but this is not necessary enough to prevent XSS. |
File: /var/www/typo3_src-4.5.17/typo3/sysext/viewpage/view/frameset.php
Vulnerability type | Warning from RIPS | Our Analysis |
Cross-Site Scripting | Line 34: Userinput reaches sensitive sink when function () is called. | Echo’s a GET value. stripslashes() is called in the _GET function and intval() is called before echo’ing. This is a false positive |
File: /var/www/typo3_src4.5.17/typo3/tce_file.php
Vulnerability type | Warning from RIPS | Our Analysis |
File Disclosure | User input reaches sensitive sink (line 83) | Path of the file to be uploaded can be modified. This is the same finding as Fortify. |
File: /var/www/typo3_src4.5.17/typo3/sysext/lowlevel/config/index.php
Vulnerability type | Warning from RIPS | Our Analysis |
File manipulation | User input reaches sensitive sink (line 3189) | False positive. This function is used to write session data and should be there. The ‘user input’ is not actual user input but a session ID which is generated in $_SERVER. |
File: /var/www/typo3_src-4.5.17/typo3/sysext/version/ws/wsol_preview.php
Vulnerability type | Warning from RIPS | Our Analysis |
Cross-Site Scripting | User input reaches sensitive sink when function main() is called.) | The workspace preview module parses a variable from GET-parameter and displays it back to the user in a standard non-persistent XSS. As mitigation, stripslashes() is applied as input validation and htmlspecialcharacters() applied as output validation. However, this is insufficient to prevent XSS. |
File: /var/www/typo3_src-4.5.17/typo3/sysext/impexp/app/index.php
Vulnerability type | Warning from RIPS | Our Analysis |
Unserialize | (User input reaches sensitive sink when function process_presets() is called. | In the import function for configuration presets, a variable from GET-parameter is passed to a vulnerable mysql query with only strip slashes as input validation. |
File: /var/www/typo3_src-4.5.17/typo3/sysext/cms/tslib/class.tslib_fe.php
Vulnerability type | Warning from RIPS | Our Analysis |
Cross-Site Scripting | User input reaches sensitive sink when function admcmd_preview() is called. | Information from cookie is printed as an error message by the die() function. die (htmlspecialchars('Request URL did not match "' . t3lib_div::getindpenv ('TYPO3_SITE_URL') . 'index.php?ADMCMD_prev=' . $inputCode . '"')). The variable $inputCode is obtained from GET-parameter. Stripslashes() is applied as input validation and htmlspecialcharacters() applied as output validation. However, this is insufficient to prevent XSS. |
File: /var/www/typo3_src-4.5.17/typo3/sysext/cms/tslib/showpic.php
Vulnerability type | Warning from RIPS | Our Analysis |
File Disclosure | User input reaches sensitive sink when function init() is called. | This function shows a picture from uploads folder in a separate window. The picture file and settings are supplied by GET-parameters: file, width, height, md5 etc. However, input validation is performed - stripslashes is first applied on the raw user input and the file path is checked for validity. (False negative) |
PHPLint
sebastian@ThinkPad-L440:/var/www/html/typo3_src-4.5.17$ find . -name '*.inc' -o -name '*.php' -print0 | \ xargs -0 -n1 -P4 php -l > phplint.output PHP Warning: declare(encoding=...) ignored because Zend multibyte feature is turned off by settings in ./typo3/sysext/fluid/Classes/Core/Widget/AbstractWidgetViewHelper.php on line 2 sebastian@ThinkPad-L440:/var/www/html/typo3_src-4.5.17$ grep -v '^No syntax errors detected' phplint.output
The result from phplint is quite remarkable: according to PHP Lint there are no syntax errors. (Erik:I would not expect any syntax errors in code anyway. Presumably PHP lint checks a bit more than just basic syntactic correctness?) This result suggests that the authors test the code with PHP Lint (or some similar tool) before deploying the tool. We could not find an official confirmation on this claim, but on the typo3 forum we found that automated building tools do apply syntax checking tools, including PHP Lint.
PHP CodeSniffer
The PHP_CodeSniffer tool analyzed the entire codebase. It was very slow and gave an impossible high amount of errors (after half an hour we stopped the tool, by then it had generated a report file of 105MB). More important, after inspecting the first lines of the report, we noticed that all errors were about wrong indentation :-). Although the documentation mentions a severity setting to ignore such errors, this setting is just a number and we could not find the meaning of this number. We will not further analyze the output of this tool.
Compare to known exploits from CVE database (false negatives)
(Erik: Very nice to check for false negs using the CVE!)
CVE Number | Description | Detected by Fortify? | Detected by RIPS? |
CVE-2013-7080 | The creating record functionality in Extension table administration library (feuser_adminLib.inc) in TYPO3 4.5.0 through 4.5.31, 4.7.0 through 4.7.16, and 6.0.0 through 6.0.11 allows remote attackers to write to arbitrary fields in the configuration database table via crafted links, aka "Mass Assignment." | no | no |
CVE-2013-7079 | Open redirect vulnerability in the OpenID extension in TYPO3 4.5.0 through 4.5.31, 4.7.0 through 4.7.16, 6.0.0 through 6.0.11, and 6.1.0 through 6.1.6 allows remote attackers to redirect users to arbitrary web sites and conduct phishing attacks via unspecified vectors. | yes | no |
CVE-2013-7078 | Cross-site scripting (XSS) vulnerability in the errorAction method in the ActionController base class in the Extbase Framework in TYPO3 4.5.0 through 4.5.31, 4.7.0 through 4.7.16, 6.0.0 through 6.0.11, and 6.1.0 through 6.1.6, when the Rewritten Property Mapper is enabled, allows remote attackers to inject arbitrary web script or HTML via unspecified input, which is returned in an error message. NOTE: this might be the same vulnerability as CVE-2013-7072. | no | no |
CVE-2013-7076 | Cross-site scripting (XSS) vulnerability in Extension Manager in TYPO3 4.5.x before 4.5.32 and 4.7.x before 4.7.17 allows remote attackers to inject arbitrary web script or HTML via unspecified vectors. | no | no |
CVE-2013-7075 | The Content Editing Wizards component in TYPO3 4.5.0 through 4.5.31, 4.7.0 through 4.7.16, 6.0.0 through 6.0.11, and 6.1.0 through 6.1.6 allows remote authenticated backend users to unserialize arbitrary PHP objects, delete arbitrary files, and possibly have other unspecified impacts via an unspecified parameter, related to a "missing signature." | yes | yes |
CVE-2013-7074 | Multiple cross-site scripting (XSS) vulnerabilities in Content Editing Wizards in TYPO3 4.5.x before 4.5.32, 4.7.x before 4.7.17, 6.0.x before 6.0.12, 6.1.x before 6.1.7, and the development versions of 6.2 allow remote authenticated users to inject arbitrary web script or HTML via unspecified parameters. | no | no |
CVE-2013-1842 | SQL injection vulnerability in the Extbase Framework in TYPO3 4.5.x before 4.5.24, 4.6.x before 4.6.17, 4.7.x before 4.7.9, and 6.0.x before 6.0.3 allows remote attackers to execute arbitrary SQL commands via unspecified vectors, related to "the Query Object Model and relation values." | no | no |
CVE-2012-6147 | Cross-site scripting (XSS) vulnerability in the tree render API (TCA-Tree) in the Backend API in TYPO3 4.5.x before 4.5.21, 4.6.x before 4.6.14, and 4.7.x before 4.7.6 allows remote authenticated backend users to inject arbitrary web script or HTML via unspecified vectors. | no | no |
CVE-2012-6145
CVE-2012-6144 |
SQL and Cross-site scripting (XSS) vulnerability in the Backend History module in TYPO3 4.5.x before 4.5.21, 4.6.x before 4.6.14, and 4.7.x before 4.7.6 allows remote authenticated backend users to inject arbitrary web script or HTML via unspecified vectors. | no | no |
CVE-2012-3531 | Cross-site scripting (XSS) vulnerability in the Install Tool in TYPO3 4.5.x before 4.5.19, 4.6.x before 4.6.12 and 4.7.x before 4.7.4 allows remote attackers to inject arbitrary web script or HTML via unspecified vectors. | no | no |
CVE-2012-3527 | view_help.php in the backend help system in TYPO3 4.5.x before 4.5.19, 4.6.x before 4.6.12 and 4.7.x before 4.7.4 allows remote authenticated backend users to unserialize arbitrary objects and possibly execute arbitrary PHP code via an unspecified parameter, related to a "missing signature (HMAC)." | yes | no |
Conclusion - Can the tools be used to check the security requirements?
Often, the output from a single tool is incomplete. Using various tools allows us to get full picture of the attack scenario. For e.g. Fortify detects cases where unvalidated user input is included in a HTTP cookie. RIPS detects the scenario where information from cookie is printed as an error message by the die() function. These two scenarios could possible be combined in a Cross-site scripting attack.
The tools (especially Fortify) does help to find all instances where input validation is required (e.g. when tainted user input reaches a vulnerable function). Without such automated tools, this task would be very tedious, much more time consuming and near impossible.
However, the tools cannot tell whether input validation had actually been done or not. More importantly, it cannot check if input validation was done correctly. To check this, other testing methods are required e.g. functional testing or penetration testing.
In conclusion, the tools definitely help to check against the security requirement (V5 Input validation). Nevertheless, it is not a panacea and still needs a skilled human reviewer to interpret the results.