SoftwareSecurity2014/Group 1/Verdict

Uit Werkplaats
Ga naar: navigatie, zoeken

Level 2B Requirements

V3 – Session Management

(Erik P: Excellent and clear argumentation in the table below!)


V3 – Session Management Verification Requirements
Requirement Verdict Justification
V3.1 - Verify that the framework’s default session management control implementation is used by the application. Fail The forum sets its own “FluxBB style!” session cookie using the user’s forum ID, two SHA-1 HMACs and the cookie expiration date in lieu of using PHPs built-in session management controls (functions.php, 354; login.php, 82).
V3.2 - Verify that sessions are invalidated when the user logs out. Pass login.php provides for action=out and will overwrite its “FluxBB style!” session cookie with junk data (login.php, 106).
V3.3 - Verify that sessions timeout after a specified period of inactivity. Pass Every script meant to be run by users (i.e. everything except “!defined(PUN)”, functions.php or specific update scripts and such), first calls common.php. This script calls check_cookie which in turn reads and parses the cookie and, amongst other things, checks the expiration date (common.php, 154; functions.php, 23).
V3.5 - Verify that all pages that require authentication to access them have logout links. Pass A non-guest user will see a logout option in the navigation bar, defined in header.php. This script is required ultimately via common.php (header.php, 210). See discussion in V3.3.
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. Pass Only functions.php reads from or writes to the session cookie. check_cookie does make the password globally available but no script writes it or makes an HMAC to write that anywhere (except the cookie checker which writes a cookie).
V3.7 - Verify that the session id is changed on login. Pass A successful login will create a new session id as described in V3.1. The HMACs use a 16-bit cookie seed that is generated during installation but this will not change between sessions (install.php, 110). Thus session ids will differ for each authentication due to the change in expiration date, which is set at login to Unix time. This value is an input to one of the HMACs (and outside of it) so decent randomness is achieved via the avalanche effect. Note that the static user_id is placed in the cookie in plaintext, which leaks information about the user.
V3.8 - Verify that the session id is changed on reauthentication. Pass If a cookie is past expiration/tampered or login fails, then the user is logged out and set to 'guest' (functions.php, 48, 62, 138, 263). No other action forces reauthentication. A reauthenticated user will have a new session id as described in V3.7.
V3.9 - Verify that the session id is changed or cleared on logout. Pass On logout the session cookie is changed to have randomized garbage data (login.php, 106). See also V3.7 for discussion

Discussion

Session-based authentication was finally implemented in FluxBB v2.0. The session management in the evaluated version, v1.5.6, allowed for some attacks. The first is a denial of service attack since a user that has disabled cookies could generate an unlimited number of sessions constrained only by how rapidly the webpage could be hit. Similarly, a user with login credentials could log in without cookies enabled repeatedly to keep generating sessions. Reference

The ‘session’ system in place in this version is unorthodox in that the session management fully relies on the security of the HMAC and not on anything like a session table maintained by the server or similar. In this regard it is questionable whether one should truly call it a session at all. Perhaps this could more accurately be described as reauthentication with a token, namely the cookie.

Session management is closely related to authentication and access control. In fact several security requirements for session managment (e.g. V3.7-V3.9) have direct consequences for the other two categories. Reviewing all three requirements together, and possibly including cryptography as well, would provide a more accurate picture of the security provided by FluxBB. Session-based authentication, introduced in FluxBB v2.0, further cemented the bind between these categories (and reduced some of the DoS attack risks described above).

V10 – Communication Security

V10 – Communication Security Verification Requirements
Requirement Verdict Justification
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. N/A By itself, the application does not contain this functionality in its source code. Even so, considering the fact that this is a PHP application, it needs to run on a web-server. Modern web-servers such as Apache and nginx have these features implemented and can be used out of the box. As a result, provided that the appropriate configurations are done, FluxBB can do these checks.
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. N/A In the case of FluxBB, the only external connections are those to web-browsers of users and the only backend connections are to the database. As specified in V10.1, the connections to web-browsers depend on the underlying web-service. On the other hand, connections to the database are not required to be secure. In "include/dblayer/mysql.php", lines 36/38 the mysql_pconnect/mysql_connect functions are used. Although these functions can be given flags to specify SSL connections, this is not the case in the source code. If the database server is not on the same machine as the PHP interpreter, then VPNs can be used to ensure a secure connection over the internet.
V10.4 - Verify that backend TLS connection failures are logged. N/A The source code handles errors through the "error" function, file "include/functions.php", line 1468 (monolithic source file...). Although the function has one argument called "message", it does not do anything with it. The function only makes sure that the page is not cached by the browser. The underlying PHP interpreter and web-server are tasked with logging errors. In "include/common.php" line 64, the "error_reporting" function is set to "E_ALL ^ E_NOTICE" (all but notice-level messages) which sets the PHP interpreter to the desired logging level.
V10.5 - Verify that certificate paths are built and verified for all client certificates using configured trust anchors and revocation information. N/A Client certificates are not used by default. FluxBB relies on a password-based authentication system only. Even so, the web-server can be configured to require the web-browser of the user to send a client-side certificate if properly configured to do so.
V10.6 - Verify that all connections to external systems that involve sensitive information or functions are authenticated. N/A Since the database is the only external system I can think of, this is covered in 10.3.
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. N/A Modern database servers provide the ability to use sepparate user accounts, each with its own set of privileges. Again, this requirement relies on the underlying system and cannot be controlled from the source code of the application itself.

Discussion

V11 – HTTP Security

V11 – HTTP Security Verification Requirements
Requirement Verdict Justification
V11.1 - Verify that redirects do not include unvalidated data. Pass (conditional) Two ways of redirecting have been checked: PHP's header() function with option "Location:" and FluxBB's redirect() function. The former is used only 11 times, only 5 of which include user input. All instances were sanitized properly. FluxBB's redirect() function is used more extensively, so all 59 instances could not be thoroughly checked. However, the majority of the instances take arguments from the files in the lang/English directory, where all of the files are of type either UTF-8 Unicode text or ASCII text, therefore the text is static and valid.
V11.2 - Verify that the application accepts only a defined set of HTTP request methods, such as GET and POST. Fail The code does not contain safeguards for this. An alternative solution would be to implement the restriction at the HTTP server level, by using appropriate .htaccess file entries. The application does not come with a default .htaccess file (unlike Joomla etc), so this has not been implemented at all.
V11.3 - Verify that every HTTP response contains a content type header specifying a safe character set (e.g., UTF-8). Pass All of the php files require the header.php which defines the Content-type as text/html and the charset as the safe utf-8.
V11.4 - Verify that the HTTPOnly flag is used on all cookies that do not specifically require access from JavaScript. Pass This is done properly, taking care of PHP < 5.2 compatibility, which did not include a special option for this. For more details see Discussion section.
V11.5 - Verify that the secure flag is used on all cookies that contain sensitive data, including the session cookie. Pass
V11.6 - Verify that HTTP headers in both requests and responses contain only printable ASCII characters. Pass (conditional) The header() function is called 41 times. All of the instances have been checked thorougly and found to be safe, apart from two. One depends on the output of PHP function gmdate(), which should be considered properly sanitized. The other uses FluxBB's public variable $pun_config to retrieve the forum's name (o_board_title) from the database. This is set in the database only by install.php, which acquires it from user input and sanitizes it using pun_trim(). No description is provided for this function. Given that it does its job correctly, and no other modification to the database has been done, this field is sanitized.

Discussion

Regarding 11.4, we had the following warning from Fortify:

Cookie Security: HTTPOnly not Set - [0 / 1] :
The program creates a cookie in functions.php at line 374, but fails to set the HttpOnly flag to true.

However, the relevant code is:

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

So, line 374 contains HttpOnly flag but 372 does not, which looks contradicting!

In Php < 5.2, HTTPOnly had not yet been defined, so it had to be given inside the cookie’s text value. This is taken care of in line 374. In Php >= 5.2 on the other hand, the last parameter of setcookie() must be set to TRUE to indicate httpOnly, which is done in line 372. So, FluxBB takes care of this properly but Fortify is not aware of this method and signals a false positive! (Erik P: Still, was it useful that Fortify drew your attention to this code, or would you have found it easily otherwise?)