SoftwareSecurity2012/Group 1/Verdict

Verdict on Security Requirements

Results:

Requirement	Result
V3.1	MAYBE/FAIL
V3.2	PASSED
V3.3	PASSED
V3.5	PASSED
V3.6	MAYBE/FAIL
V3.7	PASSED
V3.8	PASSED
V3.9	MAYBE/FAIL
V3.10	PASSED

General

On this wiki page we try to verify all security requirements of the OWASP ASV that are in Level 1 and Level 2, and rapport about it. In the end we try to give a conclusion if the requirement is met (PASS) or not met (FAIL) or if we are unsure about it.

The application does only rely on cookie for authentication purposes. It does not uses sessions in any way (Erik:Shouldn't V3.1 be a fail then? But I don't understand why you say sessions are not used, since a cookie is set upon login, as you describe below.). In the upcoming release of the software, the authentication should be implemented by using sessions.

Requirements

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

For all files except 1 the default implementation is used trough the check_cookie(), forum_setcookie() methods. I have checked this by searching all files for references to '$_COOKIE', '$_SESSION' and 'session'. The exception is in db_update.php. This file directly uses $_SESSION for storing information during exectution of the script/file instead of the default methods, which use a cookie implementation.

The file db_update.php is only used when updating the database from a older version of fluxbb to a newer version. If you run the script on the latest version the db_update.php file can handle, than you will get an error saying that you already have the latest version. I haven't checked though what happens if you don't use the latest version and how much of the script is executed then.

When examining this security requirement, I noticed that there are certain functions which remove bad characters from variables which can be modified by users like the $_COOKIE and $_SERVER. This if for example done in the functions.php:remove_bad_characters() method. This method uses a blacklist of bad characters, which is most of the time not a good thing. I don't know in which aspect this blacklisting is the only check of these variables (Erik:I don't understand the first part of this sentence)and this is also not needed to check in this requirement.

So, in conclusion, except 1 file all files use the same session management control. Because the db_update.php file can only be executed when the database is not up to date this does not need to be a security risk. Although, a better solution would be to require the file to be deleted (as well as the install.php file) before the forum software can be used. Because of this I have set the result of this requirement to 'Maybe/Fail'.

Result: MAYBE/FAIL.

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

When a user logs out, he is send to the login.php page with action='out' (login.php?action=out) After checking that a user is really logged in, and is not a guest, the cookie is reset by the code. This is called like this:

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

pun_hash(uniqid(rand()) will results in a sha-1 hash of a random int postfixed with a unique identifier of 23 chars based on the current time in microseconds. A better method of generating a random int in php would be mt_rand(), although not needed in this case I think.

The pun_setcookie() function is implemented in the following way: <source lang="php"> function pun_setcookie($user_id, $password_hash, $expire) {

   global $cookie_name, $cookie_seed;

   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);

} </source>

Eventually the forum_setcookie() method is called. <source lang="php"> function forum_setcookie($name, $value, $expire) {

   global $cookie_path, $cookie_domain, $cookie_secure;

   // Enable sending of a P3P header
   header('P3P: CP="CUR ADM"');

   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);

} </source>

The pun_setcookie() and the forum_setcookie() are the default wrappers for settings cookie values. The total result of this call is that it will reset the cookie value for the username to '1' and the password to the newly generated string. Often you see that user id 1 is used for the admin created upon installation. This had to be checked as well. After studying the installation script and running a test installation it shows that the user id 1 is created for guest account and don't have any rights. The invalidation of the cookie preformed by the logout method is thus correct.

Result: PASS.

(Erik:The cookie is unset, but, as you explain under V3.9, the session is NOT invalidated in the server, and the old cookie can still be used until the expiration time. Shouldn't V3.2 therefore be a big FAIL?)

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

The login/session information is stored in a cookie, so the cookie should be invalidated after a specified period of inactivity. The only place where a cookie check is preformed is in the functions.php:check_cookie() method. There the expiration date of the cookie contents is validated against the current time to see if the cookie is still valid. [code]$cookie['expiration_time'] > $now[/code] If the cookie is not valid anymore then the user cannot login. The other 2 checks that have to be preformed are to check if the expiration time is set correctly and to see if the expiration time is renewed after certain actions and/or upon login. The expiration date of a cookie is set to the current time + 1209600 seconds. This is the current time + 14 days. This is only done if the user checked the option to safe his password upon login. Otherwise the default timeout value is used, which is 1800 seconds. This equals 0.5 hours. After a half hour of inactivity the cookie is not valid anymore in the default configuration. Other configurations, where this value of 1800 seconds is changed by the administrator cannot be checked off course.

With every call to the check_cookie() method, the cookie is updated with an updated expiration date. The check_cookie() method is called in the includes/common.php file, which seems to be called in the header of every file which is publicly accessible.

After a specified period of inactivity the session will thus timeout, which makes that this security requirement is met.

Result: PASS.

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

Loggin out is done via the login.php file with GET parameter action set to 'out'. This means that a link to this file should be present at every page that requires login. The file header.php contains a line that, for non-guests (so for every logged in user). Displays a line in the header to logout.

Every file has a line that says

<source lang="php"> require PUN_ROOT.'header.php'; </source> except for the files that are used during installation, config, or simply contain helper functions (so they do not require authentication) so this requirement is validated. Most of these files have some sort of check to verify that that script is not ran by simply accessing the files like this: <source lang="php">

// Make sure no one attempts to run this script "directly" if (!defined('PUN')) exit;

</source> or, when the file is not located in the root of the system we can check on that.

<source lang="php"> if (!defined('PUN_ROOT')) exit('The constant PUN_ROOT must be defined and point to a valid FluxBB installation root directory.');

</source>

Result: PASS

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.

In update_db.php (which is in the main folder and thus can be reached via http://url.to.fluxbb/update_db.php ) in several occasions the session ID can be input via a GET parameter, this means that a potentially dangerous URL can be crafted when information about session ID's is known.

Additionally, when the correct parameters are given, the website will actually output your session ID.

Relevant code:

<source lang="php">

if (isset($_POST['req_db_pass']))

else if (isset($_GET['uid']))

</source> and <source lang="php">

if ($query_str != ) exit('<script type="text/javascript">window.location="db_update.php'.$query_str.'&uid='.$uid.'"</script><noscript> <meta http-equiv="refresh" content="0;url=db_update.php'.$query_str.'&uid='.$uid.'" /></noscript>');

</source> Without intense manual inspection it will be hard to see if this line of code (which is the last line of the file) is reachable but we can assume it is because in a lot of cases $query_str is set to some value. Another example of possible leakage of session ID is in the following code: <source lang="php">

<form method="post" action="db_update.php?stage=conv_users_dupe&uid=<?php echo $uid ?>"> <input type="hidden" name="form_sent" value="1" />

</source>

$uid here is the session id, again we need some parameters to be set correctly and it is hard to see how difficult that is to achieve without rigorous reading or quite some real life testing.

Because again the concern is with db_update.php (which shouldn't/doesn't work normally, and should be removed after installing), we call this verdict

Result: MAYBE/FAIL.

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

The function pun_setcookie() (in functions.php) creates a cookie with:

· User id

· Hash of password

· Expire time

· HMAC of these fields that protects integrity and authenticity of these fields. Prevents tampering by malicious user

Result: PASSED.

V3.8: Verify that the session id is changed on reauthentication.

The same as above.

Result: PASSED.

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

  Normal  0          false  false  false    EN-US  X-NONE  X-NONE

Depends on how you look at it. The system does not store anything in the database about the sessions. There is no actual way to remove sessions from the system apart from the client removing the cookie. After logout the cookie is set to the guest cookie by pun_setcookie(). If there are multiple different sessions logging out does not destroy all these sessions, it only does remove the cookie of that client. This makes the system extra vulnerable to session hijacking attempts. If an eavesdropper sniffs a cookie and sets it on her machine then there is no way to invalidate that cookie, the database does not store the valid sessions. Any proper hmac on the cookie proves the correctness of the cookie. The only way to invalidate such an hijacked session is to change to password such that the password hash in the cookie does not match anymore with the password hash in the database. Result: MAYBE/FAIL.

(Erik:I think you are a bit too kind here. I would call V3.9 a FAIL, because of the weakness you mention. I agree that `depends how you look at it', as you say above, but I think this weakness should result in a FAIL somewhere in V.*. Maybe it should be signalled under V3.2??)

V3.10: Verify that only session ids generated by the application framework are recognized as valid by the application.

As mentioned before the cookie does contain a HMAC value of its fields to prevent malicious clients to tamper with the cookie. If the client does tamper with the cookie, the cookie is set to the guest cookie. The cookie is checked for validity in check_cookie() in functions.php. Check_cookie() is invoked by common.php which is included in most php files (about 30 files require common.php). The key used by the HMAC is stored in the global variable cookie_seed stored in config.php. Config.php is written by running install.php, the value of cookie_seed is random generated and therefore most likely different on each installation. So it is impossible to change the values in the cookie such that they are still accepted by het system under the assumption that hmac-sha1 is secure.

This the chain of calls that are involved with session and cookie handling:

Require common.php -> check_cookie() -> pun_setcookie() -> forum_cookie() -> setcookie()

setcookie() is the php function call that is used to set the cookie of the client. forum_cookie() is a wrapper around setcookie(). forum_setcookie() is used by pun_setcookie() to set the cookie for the field mentioned above. Forum_setcook() is also used for a second cookie that keeps track of which topics the user wants to track, this is not related at all to security. After searching through all php files it was confirmed that the only usage of the setcookie() call is by forum_setcookie() and that forum_setcookie() only has these two applications. Futerhmore pun_setcooie() is only invoked by:(Erik:There seems to be some text missing here...)

References

https://www.owasp.org/index.php/Main_Page

https://www.owasp.org/index.php/Reviewing_Code_for_Session_Integrity

https://www.owasp.org/index.php/The_Owasp_Code_Review_Top_9

https://www.owasp.org/index.php/Some_Guidance_on_the_Verification_Process

Reflection

To see the reflection on our efforts to verify the security requirements (and the project in general) follow the link to the Reflection page.