SoftwareSecurity2013/Group 3/Code Scanning

Fortify

Fortify can group the found vulnerabilities by category.

The categories that might be of interest for the V2.1 requirement are:

• Password Management: Hardcoded password
• Privacy Violation
• System Information Leak
• Access Control: Database

For the V2.2 requirement the following categories might be of interest:

• Password Management: Password in HTML Form
• Privacy Violation: Autocomplete

There are no V7 requirements on Level 1B however Fortify is able to find some problems with cryptography so we describe them here anyway. The following categories are interesting:

• Weak Cryptographic Hash
• Insecure Randomness

V2.1 requirement

Password Management: Hardcoded password

Fortify finds 2 critical issues and 22 high priority issues of this type. All issues are entries in arrays used for localization. The critical issues are for entries with the key 'Password', and the high priority ones have keys with the word 'Password' in them. The arrays store the english messages used when asked for passwords, and no actual hardcoded passwords. Therefore, this is not an issue. We think that Fortify could be smarter here and actually check if the problems are in localization files.

Privacy Violation

Fortify finds 6 critical issues of this type. The first two of them are about an email being sent containing the password and password-key, which is used in the URL to activate an account. The email is sent to an email address from a POST-value. The password and password-key that are sent are newly generated for that address. So this information is not public to everyone, and the verification requirement is met.

The third issue is also about a password being emailed. This time the password and email address are both retreived from a POST-value. A new account is created with that email adress and password. This means that the password is sent to the owner of the new account. This is the same as in the previous two issues, so the verification requirement is again met.

The fourth issue is about privacy sensitive information that is echoed back to the user. POST-values are placed in a configuration-file which is not stored but echoed back to the user. This does not violate requirement V2.1.

The fifth issue is about privacy sensitive information that is stored to the file-system. This has again not to do with requirement V2.1.

In the final issue the database password is placed in a HTML hidden field, which means publicly available, if the HTML page is publicly available. Further manual analysis (level 2B) is needed to check whether this is the case or not. Right now we cannot tell whether the requirement is met or not.

System Information Leak

The code scanner finds one critical issue where phpinfo() is accessible. This turns out to be a admin page that is only accessible for admin users. The scanner also finds 16 low issues. Some are caused by setting the error reporting to E_ALL that could leak sensitive information:

error_reporting(E_ALL);

Another issue identified is that mysql_error's are displayed to the user. We looked at the code causing this and it only happens when PUN_DEBUG is defined. We assume there is some documentation saying that one shouldn't enable this in production.

Access Control: Database

Fortify finds 394 high issues where it thinks that access control is not done properly and that this enables an attacker to access some database information. We looked at some problem locations but most of the pages are admin pages that have some kind of access control in it.

if ($pun_user['g_id'] != PUN_ADMIN && ($pun_user['g_moderator'] != '1' || $pun_user['g_mod_ban_users'] == '0'))
	message($lang_common['No permission'], false, '403 Forbidden');

However there are also some issues with pages that aren't in the admin section and don't have access control. However further inspection of these pages showed that the intval() function was used in all these cases. So in the worse case an attacker can only choose another integer value. To see if this is a problem further analysis is needed.

Conclusion

Fortify found several issues that could point to problems with requirement V2.1. However further inspection shows that most of the pages with issues do have some kind of authentication. So most of the issues are false positives.

However we think that there is a big possibility that Fortify doesn't find pages or resources that are public but shouldn't be public. Since the code doesn't declare if a page should be public or not there is no way for Fortify to validate this. The only thing Fortify can do is to see if some information generated by php (like phpinfo()) is showed to unauthenticated users. So we think that manual verification can find some issues that Fortify didn't identify.

V2.2 requirement

Password Management: Password in HTML Form

Fority finds two critical issues that point one place in the code where two SQL passwords are echo-ed back into an admin form in admin_options.php. We think that this really is a problem. It could be that an attacker that already gained admin access can get the SQL password (and username) here, but it requires futher manual analysis to see if the random_key functions solves the problem.

<?php $smtp_pass = !empty($pun_config['o_smtp_pass']) ? random_key(pun_strlen($pun_config['o_smtp_pass']), true) : ''; ?>
<input type="password" name="form[smtp_pass1]" size="25" maxlength="50" value="<?php echo $smtp_pass ?>" />
<input type="password" name="form[smtp_pass2]" size="25" maxlength="50" value="<?php echo $smtp_pass ?>" />

Privacy Violation: Autocomplete

The code scanner finds 5 places where the password input box doesn't have autocomplete disabled. One of them is the login page. This violates V2.2 so the requirements are not met.

<input type="password" name="req_password" size="25" tabindex="2" />

Conclusion

Due to the autocompletion problems found this requirement is not met.

V7 requirements

Weak Cryptographic Hash

Forfity finds 15 Low Weak Cryptographic Hash issues. Are all due to the usage of SHA-1. Although SHA-1 is considered weak nowadays this isn't a real big issue since SHA-1 is still relatively secure. (Erik:Also, note that is also depends what the hash is used for. A hash might be weak in the sense that one can construction collisions (ie find two plaintexts with the same has), as with MD5, but that does not mean it's not suitable for say hashing passwords, where construction collisions is not a real threat)

Insecure Randomness

The scanner identifies 10 High issues related to Insecure Randomness. These issues are all due to the usage of the insecure rand() function and the less insecure mt_rand() function.

RATS

Since RATS has several options we are doing the analysis with flags "-i -r -w 3 --follow-symlinks". This gives, according to the README, the most verbose and detailed scan. Although RATS gives you, with these settings, some explanation of the possible issues it finds, they do not fit as nicely into our security requirements. However, the number of relevant issues that RATS finds is quite limited and so we will discuss whether a given finding may be relevant to our requirements.

Non-function call references

The "-r/--references" flag we use directly results in a very high number of reports containing the issue "non-function call reference". Most of these, possibly all, are irrelevant. Curiously, they are all classified as 'Medium' priority. Note that it explicitly reports that the function is not being called (even when it actually is). There could be an issue when the name is used as a function pointer to this vulnerable function.

These issues seem to occur whenever there is a variable with a name that overlaps with a dangerous function (like $link on include/parser.php:662) or when a function is being called and we include the '@' operator or even when the function is being called in the argument list of another function.

Rename

Using warning level 3, we also get the low priority issues. Chief among these is the 'rename' issue. This issue seems to occur all over the place when functions are called that operate on the filesystem. It states in the explanation that given certain circumstances there might occur a race condition. RATS states that it could not find any such circumstances but that a manual check is in order.

TOCTOU

RATS reported the existence of two possible TOCTOU (Time Of Check Time Of Use) vulnerabilities.

The first one is reported in the access to the database. If the database does not exist a new one is created and several operations are executed over that database assuming the default permissions (0666 - rw all). If during the operations and the return of the function another process changes the permissions of that file the returned database will have modified permissions. This should not result in a security issue since the original permissions are 0666 no advantage could come to change the permissions.

The second one is a verification to read the load averages from the file /proc/loadavg (admin_statistics.php). During the check and the read operation if another process changes the permissions there should be no problem. If the privileges were increased the read operation would be successful, otherwise the read operation would fail and the load averages declared has 0. No security problem could raise from this.

Function parameter validation

RATS reports a number of function calls that need their parameters checked, specifically fgets(), read() and file(). On closer inspection, none of these seem to be actually called with dangerous parameters and thus these issues do not appear to cause problems. In particular, read is never called with any parameters in fluxbb. Once more, the issue is irrelevant for the entirety of the security requirements and thus it is surely irrelevant for our security requirements.

RIPS

RIPS can be executed with different levels of verbosity and the reported vulnerabilities can be grouped into several categories. We run RIPS over the code using the higher level of verbosity (level 4). The tool reported a total of 4004 vulnerabilities including both server and client slide (it is possible to discriminate what to present).

Category	Vulnerabilities found
Code Execution	163
Command Execution	2
Header Injection	9
File Disclosure	53
File Inclusion	388
File Manipulation	39
SQL Injection	34
XSS	2961
HTTP Response Splitting	96
Possible Flow Control	239
Unserialize	20

The tool does not support Object-oriented code (Classes) yet and we were warned that analyzing OO code could result in a great number of false positives. Randomly we selected some vulnerabilities and manually checked the code in order to validate the tools output. This for categories with more than 40 vulnerabilities for the remaining ones all reported vulnerabilities were analyzed. In fact, most of the reported issues are false positives. We were not able to identify any real vulnerability in the FluxBB code and that might be due to the randomness of the selected examples. Nevertheless, the tool was running over Xampp, that was installed in the same folder as FluxBB, and that code was also analyzed. It turns out that XAMPP do not sanitize all the input.

Although we have not found any real vulnerability the tool provides some good feedback about sensitive areas of the code that require more attention. It is also a good way to check what are the dangerous functions or dangerous practices in general (echo, redirect, header, exit functions) by providing a description of the vulnerability, an example and a possible way to patch such vulnerabilities.

Since no real vulnerability was found we cannot correlate them with our requirements. Nevertheless, RIPS can be used to analyze specific files like login.php in order to find potentially dangerous code related to authentication (or other requirements).

PHPLint

We ran PHPLint on its most verbose settings and we got no useful errors. It reports a grand total of 1 FATAL ERROR, a number of ERRORs (all of which seem to be false positives) and a great deal of WARNINGs. Both the ERRORs and WARNINGs seem to be given by PHPLint whenever it does not know how to handle something or when it tries to enforce strong type checking.

None of the issues reported by PHPLint seem to have any relation to our security requirements.