Brontekst bekijken van SoftwareSecurity2013/Group 10/Code Scanning

We ran the tools on Mac osX and several Linux distributions, Mint 14, ...  OpenCart was installed on ....

{{!|Erik Poll||maybe useful to say something about the code sizeof OpenCart. The number of Fortify warnings seems lower than for the other applicaitons people are looking at, so interesting to know if OpenCart is just a smaller application, or a much better coded application which produces fewer warnings.||Group 10}}

{{!|Group 10|| The complete project has about 65,000 lines of code, we removed some uninteresting stuff like translations. That leaves about 60,000 loc. Looking at the code it seems reasonably well written, things like SQL injections are taken care of. ||Erik Poll}}

=Level 1B=
L1B.1 Perform source code scanning on the application according to the Level 1B requirements 
specified in the “Detailed Verification Requirements” section. 

L1B.2 Verify all source code scan results using either manual application penetration testing or 
code review. Unverified automated results are not considered to provide any assurance and 
are not sufficient to qualify for Level 1. 

==Verification Requirements==
===Our choice===
V2: Authentication including V7: Cryptography

====V2====
The Authentication Verification Requirements define a set of requirements for generating and handling account credentials safely. The table below defines the corresponding verification requirements that apply for each of the four verification levels. 

'''V2.1''' Verify that all pages and resources 
require authentication except those 
specifically intended to be public

'''V2.2''' Verify that all password fields do not 
echo the user’s password when it is 
entered, and that password fields (or 
the forms that contain them) have 
autocomplete disabled.

====V7====
The Encryption Verification Requirements define a set of requirements that can be used to verify an application’s encryption, key management, random number, and hashing operations. Applications should always use FIPS 140-2 validated cryptographic modules, or cryptographic modules validated against an equivalent standard (e.g., a non-U.S. standard). The table below defines the corresponding verification requirements that apply for each of the four verification levels.

===Findings of Fortify===

{{hilite|(Erik: Did you check for some of the warnings by Fortify listed below, in particular the ones relevant for V2 and V7, whether they are false or true positives?As note write above, "Unverified automated results are not considered to provide any assurance and are not sufficient"...)}}

'''Response''' We checked the warnings presented by Fortify and added an additional column in the table below. We only did the these verifications on the V2 and V7 requirements.

{{hilite|(Erik: I was intrigued by the "not applicable requirement" comment, that you give below for some Fortify and some RATS warnings. Do you mean by this that there is no security requirement that this warning is relevant for? Or did you mainly consider V2 and V7 here? In the former case, note that say TOCTOU is a real potential security vulnerability,so if it is not covered by any ASVS requierement this would suggest that these requirements are incomplete.)}}

'''Response''' The "No applicable requirement" comment came forth due to the fact that we found no applicable level 1 security requirement for these issues. At first, we compared the issues that were found by Fortify with the security requirements level 1 for the: V2, V3, V4, V5, V6, V8, V9 and V11 requirements. Within these chosen requirements, there was no level 1 security requirement that was applicable and thus the issues were rated "No applicable requirements". Later, we found out that we only had to do 2 verification requirements and we reduced the list to only V2 and V7.

{| class="wikitable" border="1"

! Relevance !! Amount !! Description !! Output !! Location || Applicable requirement || True / False Positive 
|-
| <span style="background:#FF0000">Critical</span> || 3 || Insecure Submission of password || The form in [location] submits a password as part of an HTTP GET request on line 64, which will cause the password to be displayed, logged, and stored in the browser cache || index.html:1197, forms.hrml:64, css-tests.html:943 || V2.2 and V8 || False positive
|-
|  || 1 || Password in HTML form || Populating password fields in an HTML form could result in a system compromise || forms.html:64 || V2.2 and V8 || False positive
|-
| <span style="background:#FF7F50">High</span> || 5 || Overly Broad Path (Cookie Security) || A cookie with an overly broad path can be accessed through other applications on the same domain.|| currency.php:45, index.php:182, index.php:203, session.php:10, startup.php:15 || V3.6
|-
|  || 2 || Code injection || The file index.php interprets unvalidated user input as source code on line 56. Interpreting user-controlled instructions at run-time can allow attackers to execute malicious code || index.php:56, json.php:126 || V5.2
|-
|  || 9 || Header Manipulation || Including unvalidated data in an HTTP response header can enable cache-poisoning, cross-site scripting, cross-user defacement, page hijacking, cookie manipulation or open redirect. || crossdomain.php:5, (4) currency.php:45, (2) index.php:182, (2)index.php:203 || V4.1
|-
|  || 70 || Insecure Randomness || The random number generator implemented by rand() cannot withstand a cryptographic attack. || affiliate.php, contact.php, customer.php, download.php, forgotten.php, install.php, graph-types.html, manual.php, order.php, paymate.php. pie.html, pp_pro_uk.html, product.php, realtime.php, return.php, stacking.html, thresholding.html, user.php || V7 || True positive
|-
|  || 1 || Empty Password || Empty passwords can compromise system security in a way that cannot be easily remedied || fraud.php:279 || V2.1 || False positive
|-
|  || 68 || Hardcoded Passwords || False Positives || || V5.2
|-
|  || 3 || Possible global variable overwrite || The program invokes extract(), which can overwrite global variables and might open the door for attackers || controller.php:56, loader.php:83, template.php:9 || V4.1
|-
|  || 3 || Autocomplete || The form uses autocompletion, which allows some browsers to retain sensitive information in their history || css-tests.html:943, forms.html:64, index.html:1197  || V2.2 || True positive
|-
|  || 1 || Weak XML Schema, Unbounded Occurences || Setting a maxOccurs value to unbounded can lead to resources exhaustion and ultimately a denial of service || modification.xsd:10 || No applicable requirement
|-
|-
| <span style="background:#FFD700">Medium</span> || 7 || File Upload ||  Permitting users to upload files can allow attackers to inject dangerous content or malicious code to run on the server || download.php:515, filemanager.php:480, forms.html:95, installer.php:80, installer.php:87, order.php:2334, product.php:723 || V5.2
|-
| <span style="background:#FFFF00">Low</span> || 1 || Commadn Injection || Calls exec() to execute a command.  This might allow an attacker to inject malicious commands. || cli_install.php:300 || V2.1, V4.1 and V5.2
|-
|  || 4 || Cookie not send over SSL || The program creates a cookie without setting the secure flag to true || crossdomain.php:5, currency.php:45, index.php:182, index.php:203 || No applicable requirement (perhaps V3.6)
|-
|  || 6 || HTTP only not set || The program creates a cookie in crossdomain.php at line 5, but fails to set the HttpOnly flag to true || crossdomain.php:5, currency.php:45, index.php:182, index.php:203, session.php:10, startup.php:15 || No applicable requirement (perhaps V3.6)
|-
|  || 5 || Persistent Cookie || Storing sensitive data in a persistent cookie can lead to a breach of confidentiality or account compromise. ||  currency.php:45, index.php:182, index.php:203, session.php:10, startup.php:15 || No applicable requirement (perhaps V8.1)
|-
|  || 26 || Crosside Request Forgery || The HTTP request at ajax.html line 90 must contain a user-specific secret in order to prevent an attacker from making unauthorized requests. || ajax.html, async_json.html, css-tests.html, forms-responsive.html, forms.html, index.html, static_async_json.html || V4.1
|-
|  || 11 ||Hardcoded Domain in HTML || References a script using a hardcoded domain name. If attackers compromise the domain, they will have malicious code on this page. || Refer to google code, one to Twitter || No applicable requirement
|-
|  || 2 || JS hijacking: Vulnerable Framework || Applications that use JavaScript notation to transport sensitive data can be vulnerable to JavaScript hijacking, which allows an unauthorized attacker to read confidential data from a vulnerable application. || ajax.html:90, ajjax.html:124 || No applicable requirement
|-
|  || 3 || Password in Comment || Hardcoded passwords can compromise system security in a way that cannot be easily remedied || cli_install.php:14, cli_install.php:17, fraud.php:39 || No applicable requirement (perhaps V8.1)
|-
|  || 1 || Php Design flaw, race condition || The PHP open_basedir configuration option contains a design flaw that leaves it vulnerable to file access race conditions, which can allow an attacker to circumvent access control checks on the file system. || php.ini:12 || V2.1 and V4.1 || True positive
|-
|  || 6 || System Information Leak || The program might review system data or debugging information with a call to error_reporting().  The information revealed by error_reporting() could help an adversary form a plan of attack || cli_install.php:24, index.php:3, mysql.php:55, postgre.php:46, startup.php:3 || V8.1
|-
|  || 83 || Weak Cryptographic Hash || Weak cryptographic hashes cannot guarantee data integrity and should not be used in security-critical contexts || affiliate.php, authorienet_aim.php, confirm.php, contact.php, customer.php, download,php, forgotten.php, fraud.php, install.php, json.php, liqpay.php, login.php, mail.php, manual.php, moneybookers.php, order.php, paymate.php, pp_pro_uk.php, product.php, return.php, twocheckout.php, user.php || V7 || True positive

|}

===Findings of RATS===
{| class="wikitable" border="1"

! Relevance !! Amount !! Description !! Output !! Location || Applicable requirements
|-
| <span style="background:#FF7F50">High</span> || 6 ||Issue: fopen || Argument 1 to this function call should be checked to ensure that it does not come from an untrusted source without first verifying that it contains nothing dangerous  ||home.php:48,65,82,99,116, klarna_account.php:359, klarna_invoice.php:233, error_log.php:60, cli_install.php:246,285, step_3.php:39,76, cache.php:32, log.php:12, mail.php:126 || V2.1, V4.1 and V5.2
|-
|  || 1 || Issue: eval || Argument 1 to this function call should be checked to ensure that it does not come from an untrusted source without first verifying that it contains nothing dangerous || index.php:56 || V2.1, V4.1 and V5.2
|-
|  || 1 || Issue: mail|| Arguments 1, 2, 4 and 5 of this function may be passed to an external program. (Usually sendmail). Under Windows, they will be passed to a remote email server. If these values are derived from user input, make sure they are properly formatted and contain no unexpected characters or extra data. || mail.php:148,150 || V6.1
|-
| <span style="background:#FFD700">Medium</span> || 3 || Issue: is_file ||  A potential TOCTOU (Time Of Check, Time Of Use) vulnerability exists|| filemanager.php:213,307,391 || No applicable requirement
|-
|  || 1 || Issue: fsockopen ||  Argument 1 to this function call should be checked to ensure that it does not come from an untrusted source without first verifying that it contains nothing dangerous. || mail.php:153 || V2.1, V4.1 and V5.2
|-
|  || 1 || Issue: is_writeable ||  A potential TOCTOU (Time Of Check, Time Of Use) vulnerability exists|| cli_install.php:186 || No applicable requirement
|-
|  || 1 || Issue: is_dir ||  A potential TOCTOU (Time Of Check, Time Of Use) vulnerability exists|| filemanager.php:224 || No applicable requirement
|}

== Discussions ==

=== True / False positives ===
Fortify seems to have many false positives, because it does not takes into account the context in which a function is used. In OpenCart, Fortify found many issues in the test files, where a few insecure functions where used. If the author is smart, he would never put these test files online and there would be no real danger. However, it is good that Fority still finds these issues, because it gives some kind of "trust" in the software.