SoftwareSecurity2013/Group 10/Verdict

Overview

Security requirements (V2,V7) relevant for level 2B:

Requirement	Verified
V2.1
V2.2
V2.3
V2.4
V2.5
V2.6
V2.7
V2.8
V2.9
V2.10
V2.11
V2.12
V2.13
V2.14
V7.1
V7.2
V7.3
V7.4
V7.5
V7.6

V2

V2.1

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

Manual

PASS All relevant controller pages start with the code: "if (!$this->customer->isLogged()) {", which checks if the user is logged in. In regard to the findings of Fortify: the exec function is only used in the install directory, which is not such a great security issue, because any good programmer (Erik: I guess you mean sys-admin rather than programmer) would definitely remove this directory when OpenCart is installed.

Fortify

Relevance	Amount	Description	Output	Location	Applicable requirement	True / False Positive
High	1	Empty Password	Empty passwords can compromise system security in a way that cannot be easily remedied	fraud.php:279	V2.1	False positive
Low	1	Command 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
Low	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

RATS

Relevance	Amount	Description	Output	Location	Applicable requirement	True / False Positive
High	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
High	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
Medium	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

(Erik:I assume you checked where this first argument came from and decided that these warnings were false positives, given that you give this requirement a pass in the manual evaluation.)

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.

Manual

FAIL Concerning the true positive of fortify below, in our quick scan of the output of fortify this seemed a real issue. It is a security issue but it is located in the documentation of font-awesome and therefore not a real risk for the application we are auditing. (Erik: These last comments are not so clear to me, but i guess I lack some context. what is font-awesome and why is it not a real risk, if it does mean that autocomplete is not disabled?)

However this requirement is not met, as autocomplete on the login-form is not-disabled, and the the password the user entered is returned to him as in a value of the input field. This was found in the login form of the main site.

Fortify

Relevance	Amount	Description	Output	Location	Applicable requirement	True / False Positive
Critical	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
Critical	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
High	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	False positive

V2.3

Verify that if a maximum number of authentication attempts is exceeded, the account is locked for a period of time long enough to deter brute force attacks.

Manual

FAIL OpenCart does not handle the amount of tried logins, therefore it cannot exceed. An OpenCart account can be brute forced, as long as server has no countermeasures implemented.

V2.4

Verify that all authentication controls are enforced on the server side.

Manual

PASS None of the JavaScript code handles authentication controls. Therefore it is assumable that all authentication controls are enforced server side.

V2.5

Verify that all authentication controls (including libraries that call external authentication services) have a centralized implementation.

Manual

PASS All the user authentication can be traced back to a single user class that does the login procedure, therefore there is one centralized implementation of the user authentication. There is also some sort of "AIM" integration, opencart uses the web api for authenticating it's "AIM" users.

V2.6

Verify that all authentication controls fail securely.

Manual

PASS Authentication is done in the User class. When the authentication fails, 'false' is returned, no information is leaked. However, the SQL query uses the SHA1 function of the database, which may leak the password to logs. (Erik: This last sentence is a bit cryptic to me - i guess if you know the code it is clear what you mean :-) )

V2.7

Verify that the strength of any authentication credentials are sufficient to withstand attacks that are typical of the threats in the deployed environment.

Manual

FAIL Brute forcing is possible, because the password has to be at least 4 characters and at most 20 characters, which severely decreases the number of possibilities. Moreover, the username offers no protection at all, because email addresses are used for this. Additionally, it is possible to change the password without filling in the old password, which makes it very vulnerable when a session is hijacked.

V2.8

Verify that all account management functions are at least as resistant to attack as the primary authentication mechanism.

Manual

PASS All account management functions require a login, which can be seen as primary authentication mechanism. Therefore are all account management functions are as attack resistant as the primary authentication mechanism. However, no re-authentication is required to use account management functions, therefore could session hijacking be a risk.

V2.9

Verify that users can safely change their credentials using a mechanism that is at least as resistant to attack as the primary authentication mechanism.

Manual

FAIL For changing your password the application does not require to enter the current password.

V2.10

Verify that re-authentication is required before any application-specific sensitive operations are permitted.

Manual

FAIL One could consider placing an order Application sensitive. For this no re-authentication is required. (Erik: Of course, it is highly subjective what is a sensitive operation. I can imagine that users would find it a hassle to have to re-authenticate before ordering, and you might not require that in a web-app like this.)

V2.11

Verify that after an administratively-configurable period of time, authentication credentials expire.

Manual

FAIL There is no such facility in Opencart.

V2.12

Verify that all authentication decisions are logged.

Manual

FAIL Failed authentication attempts are not logged.

V2.13

Verify that account passwords are salted using a salt that is unique to that account (e.g., internal user ID,account creation) and hashed before storing.

Manual

PASS All passwords are salted with a random value therefore all salts are presumably unique. Passwords are salted and hashed before storing. The sha1 hashing algorithm is used, and no bit-stretching techniques. Also the salt is prepended to the password instead of appended. Also the salt generation is weak due to a weak random generator. In practice this is probably not a real issue, (Erik: Indeed, iI doubt that a weak random number generation is an issue for salts. As long as the chance of having the same salt value for multiple users (which would help in dictionary attacks) is neglible, there is no problem. And I guess even a very weak RNG would ensure this.) but with all the other small issues it is probably only border-line secure.

V2.14

Verify that all authentication credentials for accessing services external to the application are encrypted and stored in a protected location (not in source code).

Manual

FAIL The passwords for the optional smtp and ftp server are stored plaintext in the database. Tokens for Google Checkout and other payment services are also plaintext in the database.

V7

V7.1

Verify that all cryptographic functions used to protect secrets from the application user are implemented server side.

Manual

PASS The Javascript does not implement cryptography and we assume, that the libraries used do not implement cryptographic functions.

V7.2

Verify that all cryptographic modules fail securely.

Manual

PASS There is a 'config_error_display' variable that, if set to 0, causes all errors to be hidden from front-end users. The application provides it's own php.ini, which has the variable 'display_errors' turned off by default. So assuming the application is setup correctly, this requirement is verified.

V7.3

Verify that access to any master secret(s) is protected from unauthorized access (A master secret is an application credential stored as plaintext on disk that is used to protect access to security configuration information).

Manual

PASS The config file that stores this information is not accessible, but largely depends on server settings. Unless an attacker is able to run PHP code this file us not accesible through the application, but we assume this is unfeasible as it is not part of the scope V2 and V7 (and would result in much more vulnerabilities).

V7.4

Verify that password hashes are salted when they are created.

Manual

PASS Each password hash gets salted.

Fortify

The hash function is weak, even though this belongs to V7.7, we think that it is relevant, even on level 2B.

Relevance	Amount	Description	Output	Location	Applicable requirement	True / False Positive
Low	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

(Erik:A weak hash may allow an attacker to create two plaintexts with the same hash, ie a hash collision, and exploit this somehow. But for hashing passwords this is not an attack possibility. What would be a threat if the hash is reversible.)

V7.5

Verify that cryptographic module failures are logged.

Manual

PASS An error file is created which logs cryptographic module failures.

V7.6

Verify that all random numbers, random file names, random GUIDs, and random strings are generated using the cryptographic module’s approved random number generator when these random values are intended to be unguessable by an attacker.

Manual

FAIL The random function from php (rand()) is used for generating the salt. Depending on the point of view and the knowledge of the attacker, it could be classified as "intended to be unguessable by an attacker". The other random function used, is mt_rand(), which is either used in combination with md5 (md5(mt_rand())) or sha1 and uniqid (sha1(uniqid(mt_rand(), true))). The problematic aspect here is the entropy of mt_rand() which is only Parsen mislukt (MathML met SVG- of PNG-terugval (aanbevolen voor moderne browsers en toegankelijkheidshulpmiddelen): Ongeldig antwoord ("Math extension cannot connect to Restbase.") van server "https://en.wikipedia.org/api/rest_v1/":): {\displaystyle 2^{31} -1} bits. These "random" values are used for passwords and tokens. A general problem can be seen in the leakage of a seed, because none of the used random functions gets called with a seed.

Fortify

Relevance	Amount	Description	Output	Location	Applicable requirement	True / False Positive
High	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