SoftwareSecurity2013/Group 12/Verdict

Verdict

For FluxBB 1.5, Fortify gives 2418 warnings with levels critical (254), high (1173), medium (123) and low (868). For our verifications V3, V10 and V11 we inspected all the categories, but we found the most usefull warnings in the folders Cookie Security, Header Manipulation and Open Redirect. There were about 5-10 different warnings that seemed to be useful at first sight. During manual inspection we concluded most of these warnings were false positives or were not really relevant for the requirement itself. In the table below an overview of the verdicts is presented. Furthermore, this page contains an explanation for each verdict.

Requirement\Verdict	Satisfied	Not satisfied	Not sure	Out of scope	Didn’t have enough time
V3.1 Verify that the framework’s default session management control implementation is used by the application.		x
V3.2 Verify that sessions are invalidated when the user logs out.	x
V3.3 Verify that sessions timeout after a specified period of inactivity.	x
V3.5 Verify that all pages that require authentication to access them have logout links.					x
V3.6 Verify that the session id is never disclosed other than in cookie headers; particularly in URLs, error messages, or logs.			x
V3.7 Verify that the session id is changed on login.	x
V3.8 Verify that the session id is changed on re-authentication.	x
V3.9 Verify that the session id is changed or cleared on logout.	x
V10.1 Verify that a path can be built from a trusted CA to each TLS server certificate, and that each server certificate is valid.				x
V10.3 Verify that TLS is used for all connections that are authenticated or that involve sensitive data or functions.		x
V10.4 Verify that backend TLS connection failures are logged.		x
V10.5 Verify that certificate paths are built and verified for all client certificates using configured trust anchors and revocation information.				x
V10.6 Verify that all connections to external systems that involve sensitive information of functions are authenticated.		x
V10.7 Verify that all connections to external systems that involve sensitive information or functions use an account that has been set up to have the minimum privileges necessary for the application to function properly.				x
V11.1 Verify that redirects do not include unvalidated data.					x
V11.2 Verify that the application accepts only a defined set of HTTP request methods, such as GET and POST.		x
V11.3 Verify that every HTTP response contains a content type header specifying a safe character set (e.g., UTF-8).			x
V11.4 Verify that the HTTPOnly flag is used on all cookies that do not specifically require access from JavaScript.	x
V11.5 Verify that the secure flag is used on all cookies that contain sensitive data, including the session cookie.		x
V11.6 Verify that HTTP headers in both requests and responses contain only printable ASCII characters .		x

Session Management

V3.1 Verify that the framework’s default session management control implementation is used by the application

Fortify cannot help in giving an appropriate warning related to this. The main (default) way that the PHP framework uses for session management control is the use of sessions. In the case of fluxBB, the developers' choice has been to only maintain a cookie in the client with minimal information including the userID and several hashes. Cookies are build as following:

forum_setcookie($cookie_name, $user_id.' | '.forum_hmac($password_hash, $cookie_seed. '_password_hash').'|'.$expire.'|'.forum_hmac($user_id.'|'.$expire, $cookie_seed.'_cookie_hash'), $expire);

FluxBB does not use sessions for its normal users (only for administrators that update the database), and since sessions are the default session management control of PHP, the requirement is not met.

V3.2 Verify that sessions are invalidated when the user logs out

Again this verification point is not helped by any of the tools we may use as static code analyzers. To perform this check we noticed that when the user logs out he is referred to the login.php page with several GET parameters. The “action” parameter is “out” which means that the user wants to log out. Among other irrelevant actions like removing the user from the “users online” list, the cookie is changed at the client. Precisely the ID of the user in the cookie is set to 1 which means he now is a guest visitor. Instead of the password hash the developer puts the SHA-1 hash of a random integer and the expiration time is extended with 1 year from this moment. Cookies are used to store sessions in FluxBB, they are invalidated so the requirement is thus met.

V3.3 Verify that sessions timeout after a specified period of inactivity

This again is only verifiable through manual inspection and Fortify does not help. In fluxBB’s session tracking method there is no way of detecting user inactivity since cookies do not register any information of the user interacting with the website. However an approximate solution to this is that the cookie will expire after 14 days (1209600sec.) if the user logs in and checks “Keep me logged in”. Cookies are expired after 14 days and thus the requirement is met.

V3.5 Verify that all pages that require authentication to access them have logout links

This feature again is one which requires solely manual inspection. The index.php where the forum is first launched contains at line 182 a “require” statement to include file “header.php”. This file will display at all times the header components among which is the Login/Logout link depending on whether the user is already logged in or not. This conditional check is done at the file “header.php” line 198. This resembles the “master page” design pattern used in ASP.NET. The requirement is met for index.php, but we do not know for sure for all pages. However, we assume that the “header.php” is included in every response and therefore we assume the requirement is met. (Erik: Note that it is probably easier - and more natural - to verify this requirement using a dynamic analysis (checking an installation of the web-app) than inspecting the code. That was out-of-scope,of course, for this assignment.)

V3.6 Verify that the session id is never disclosed other than in cookie headers; particularly in URLs, error messages, or logs. This includes verifying that the application does not support URL rewriting of session cookies

The sessionID FluxBB uses to authenticate a user, which is a hash as stated in V3.1, is never disclosed other than by the cookie header.

FluxBB only uses a PHP session whenever FluxBB is updated to a new version, with db_update.php. A custom session ID is created with

$uid = pun_hash($req_db_pass.'|'.uniqid(rand(), true));

This uid is indeed echo’d into an URL with

form method="post" action="db_update.php?stage=conv_users_dupe& uid=<?php echo $uid ?>">

and

<script type="text/javascript">window.location= "db_update.php'.$query_str.'&uid='.$uid.'"</script>

It is possible to rewrite URL’s, but only for the db_update.php file. However, an attacker will only be able to update the database, or exploit this particular db_update.php file. Since there is a different session created, the cookies from an administrator to the normal forum are not disclosed. Technically speaking the requirement is not met, although the idea of the requirement is probably met.

V3.7 Verify that the session id is changed on login

Since we have no PHP session ID in fluxBB the solution is of course to set/modify the cookie and make it indicate that the successfully authenticated user is now not a guest. This is performed in the login.php file line 82 where the cookie is given the values of the user ID, the SHA-1 hash of his password and a new expiration time. The requirement is thus met.

V3.8 Verify that the session id is changed on re-authentication

This is done using function “check_cookie($user)” defined in file “functions.php” which is utilized in file “common.php”. File “common.php” (the one in ~/include/ folder) is mnemonically called so because it is included in all the pages of the program. In line 70 of “functions.php” the cookie is refreshed with a new expiration time. Since the expiration date has changed, the complete cookie value and thus the ID has changed. The requirement is thus met.

V3.9 Verify that the session id is changed or cleared on logout

When the user logs out he is referred to the login.php page with several GET parameters. The “action” parameter is “out” which means that the user wants to log out. Among other irrelevant actions like removing the user from the “users online” list, the cookie is changed at the client. Precisely the ID of the user in the cookie is set to 1 which means he now is a guest visitor. Instead of the password hash the developer puts the SHA-1 hash of a random integer and the expiration time is extended with 1 year from this moment. This is done using the following statement:

pun_setcookie(1, pun_hash(uniqid(rand(), true)), time() + 31536000);

The requirement is thus met. (Erik:You describe that the cookie is changed at the client, but it is not so clear if it is also cleared at the server, ie. that the old cookie can no longer be used to continue the session.

Communication Security

V10.1 Verify that a path can be built from a trusted CA to each Transport Layer Security (TLS) server certificate, and that each server certificate is valid.

We are only inspecting the source code of the website, so there are no server certificates to validate. This requirement is out of the scope and can not be yet checked.

V10.3 Verify that TLS is used for all connections (including both external and backend connections) that are authenticated or that involve sensitive data or functions.

TLS is optional for some connections, but not enforced by FluxBB. For instance, connecting to the backend database is possible without the use of TLS. This requirement is thus not met, since not all of the connections are obliged to use TLS.

V10.4 Verify that backend TLS connection failures are logged.

Upon manual inspection there appears to be no logging functionality for failed TLS connections. This requirement is thus not met.

V10.5 Verify that certificate paths are built and verified for all client certificates using configured trust anchors and revocation information.

We cannot deduct from the source code if certificate paths are built and verified for all client certificates using configured trust anchors and revocation information. This requirement is out of the scope and can not be yet checked.

V10.6 Verify that all connections to external systems that involve sensitive information or functions are authenticated.

The only connections that were made that we could find with manual inspection were the connection to the mail server and the connection to the SQL server. Both connections could definitely involve sensitive information. The connection to the mail server supports SSL for authentication, it can be used, but this is not enforced. FluxBB authenticates itself to the SQL server, but the SQL does not authenticate itself to FluxBB. The requirement is thus not met.

V10.7 Verify that all connections to external systems that involve sensitive information or functions use an account that has been set up to have the minimum privileges necessary for the application to function properly.

For this requirement the website would have to be running on a server with connections to external systems, so that we can inspect the accounts that are used by that systems, but we are only inspecting the source code of the website. This requirement is out of the scope and cannot be checked yet.

HTTP Security

V11.1 Verify that redirects do not include unvalidated data.

Fortify triggers warnings related to unvalidated data used on redirect function which could lead to different kind of attacks such as phishing and XSS. The tool classifies the warnings according the type of attack and it is easy to search and look through the vulnerable piece of code. Fortify can check for unvalidated data in redirects, for example when the filter is set up to ‘Any attribute contains redirect’. 8 lines produce warnings, of which 3 are critical, 3 are high and 2 are low.

The critical lines are:

<meta http-equiv="refresh" content="<?php echo $pun_config['o_redirect_delay'] ?>;URL=<?php echo $destination_url ?>" />

<?php echo $message.'

<a href="'.$destination_url.'">'.$lang_common['Click redirect'].'</a>' ?>

header('Location: '.str_replace('&', '&', $destination_url));

Upon manual inspection it seems that these 8 warnings are all false positives. FluxBB uses its own validator, and it is defined as

$destination_url = preg_replace('%([\r\n])|(\%0[ad])|(;\s*data\s*:)%i', , $destination_url);

Upon manual inspection, there appears to be 241 uses of redirects in the code. Fortunately, the validator is hardcoded in the function of redirect(), so every call to that function satisfies the requirement. However, it could be that there are other uses of redirects in the code that do not use the function redirect(). Inspecting all possible ways to redirect a page would be too time consuming. It is possible that the requirement is met, however we are not entirely positive because guaranteeing this would be too time expensive.

V11.2 Verify that the application accepts only a defined set of HTTP request methods, such as GET and POST.

In order for the application to check the PHP request type it has to check the "REQUEST_METHOD" field of the global $_SERVER variable. This way we can see if the issued request is part of the predefined list of allowed request methods (e.g. GET or POST), otherwise the request will not be served.

$_SERVER['REQUEST_METHOD']

This requirement is not taken into consideration in FluxBB.

The requirement is thus not met.

V11.3 Verify that every HTTP response contains a content type header specifying a safe character set (e.g., UTF-8).

In header.php the line header('Content-type: text/html; charset=utf-8') is specified. We expect the header to be in every response, so we expect that the charset will be utf-8.

In that case, the requirement will be met.

V11.4 Verify that the HTTPOnly flag is used on all cookies that do not specifically require access from JavaScript

Fortify can check whether the HTTPOnly flag is set in the setcookie() function. In the following example, FluxBB is checking the version of PHP used on the server. Since the versions older than 5.2.0 do not support HTTPOnly flag, the setcookie function uses the string HTTPOnly not as a parameter flag itself but as an internal flag and appends to the cookie path. Fortify could not detect this conditional version checking thus led to a false positive. Another misinterpretation of Fortify is that it may lead to false negatives. The programmers of fluxbb have written two wrappers for the setcookie() function (that call setcookie() with a reduced number of parameters for practicality). These wrapper functions are safe with respect to HTTPOnly flag because they don’t take it as a parameter but in case they did, Fortify would not give a warning for that.

if (version_compare(PHP_VERSION, '5.2.0', '>='))
   setcookie($name, $value, $expire, $cookie_path, $cookie_domain, $cookie_secure, true);
else
   setcookie($name, $value, $expire, $cookie_path.'; HttpOnly', $cookie_domain, $cookie_secure);

This requirement is thus met.

V11.5 Verify that the secure flag is used on all cookies that contain sensitive data, including the session cookie.

Fortify was able to trigger a warning related to "sensitive data in persistant cookies". However, the warning is not completely related to the issue about secure flag since it actually warns about "Storing sensitive data in a persistent cookie can lead to a breach of confidentiality or account compromise".

The “secure flag” is the sixth parameter of the setcookie() function in php. We can see in the include/install.php file there is a script that generates the configuration file for fluxBB. In this script the global variable $cookie_secure is set to false (0). The following lines, which trigger other types of warnings as well, define the content of the cookie and in both cases of the conditional specify the parameter "$cookie_secure". However the tool does not show if there is explicit validation about the actual value passed to the function and whether it is defined or not.

if (version_compare(PHP_VERSION, '5.2.0', '>='))
   setcookie($name, $value, $expire, $cookie_path, $cookie_domain, $cookie_secure, true);
else
   setcookie($name, $value, $expire, $cookie_path.'; HttpOnly', $cookie_domain, $cookie_secure);

In FluxBB it is used as a wrapper of the setcookie() function and this wrapper doesn't modify the $cookie_secure parameter. Thus everytime a cookie is set, the $cookie_secure is equal to 0.

The requirement is thus not met.

V11.6 Verify that HTTP headers in both requests and responses contain only printable ASCII characters.

There are no explicit warnings about the verification of printable ASCII characters, however the warnings related to header manipulation were useful in order to identify -indirectly- whether the functions related with the manipulation of the headers validate the printable charset for the parameters within the header definition.

Fortify was able to trigger warnings about a lack of validation of the content within the HTTP headers. These warnings seem to check if actual content is a valid value. One of the warnings is "Header Manipulation (Input Validation and Representation, Data Flow)" and the reason given by the tool is because "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". Indeed the tool is quite useful to check correct values within the headers, but there is no explicit verification or details from the tool whether the validation also includes the printable charset ASCII for the content. Thus, a manual verification has been done in order to identify whether the content of the header is validated within the code. We have verified that even though the warnings are not related to the printable charset ASCII itself, they were useful to identify the lines of code where the headers are manipulated. The following table shows the lines where the warnings are triggered.

File name	Line Number	Unchecked variable on header
functions.php	1293	destination_url
functions.php	348	name, value, expire, cookie_path, cookie_domain, cookie_secure
functions.php	350	name, value, expire, cookie_path, cookie_domain, cookie_secure
viewforum.php	38	cur_forum
viewtopic.php	57	first_new_post_id
viewtopic.php	63	id
viewtopic.php	75	last_post_id

As an example of this issue, the following lines of code show warning trigger "Header manipulation" since there is no validation of the variable $first_new_post within the header content.

if ($first_new_post_id)
{
    header('Location: viewtopic.php?pid='.$first_new_post_id.'#p'.$first_new_post_id);
    exit;
}

The requirement is thus not met.