Software Security/Group 7/Verdict

Introduction

We need to check the section of OWASP on access control.

We have 13 requirements to check. It's sometimes difficult to see the difference between requirements and that is why we have merged some sections. It's to reduce the overlapping between the different requirements.

In access control we need to check if people without the proper status can access to protected data. We trust the file Auth.php which gives authorization for each person that uses the forum.

It's the job of group 9 (V2 : Authentication) to check if there are no vulnerabilities in the file Auth.php.

Final verdict

Verify that users can only access data files for which they possess specific authorization.

Status: Pass

In this part we check if the php functions that operate on data files have sufficient access rights.

Particularly we try to find the calls of fwrite in files (to identify what the data files could be):

   - admin_db_utilities.php: fwrite in a comment
   - admin_user.php: set avatar file, it is an admin function
   - template_file_cache.php: the function is pparse(), it calls fwrite; it's called in several files.
        *pparse() called in admin files, which check if the user is admin.
        *faq.php calls via common.php

• All admin functions (files) are safe. All of them include "pagestart.php" which performs a check for admin rights (line 44) and dies on failure.
• All admin files meant for inclusion only use a symbol (IN_ADMIN) to check if they are actually being included and not called directly.
• The config.php file is protected, because we suppose the admin will configure the webserver correctly, and the install script will configure it for the admin.

Verify that direct object references are protected, such that only authorized objects are accessible to each user

Status: Uncertain (hard to decide as there are too many error messages are false positive)

PHP is not a language with objects, it may be strange for us to need to check that, we have looked up the definition in the website of OWASP: "The application uses unverified data in a SQL call".

Fortify detects errors using tainted value analysis.

It reported some problems about direct object references but all of them we have checked are either false positives or require the user to be an admin to perform the actual exploit.

Fortify false positives

A file uses $_GET to have an 'id' and does a SQL call with that id in the SQL table of registered users. Fortify says it's a tainted value, but the function of that file is actually to see the information of any person registered on the site, which will naturally need the id of that person.

A file uses $_POST and registers the value in an array and thereafter uses another value in the same array to do a SQL call. Fortify considers the whole arra to be tainted as opposed to specific indices in the array. We consider this a false positive.

In admin_smiles.php, line 100:

 $smile_pak = ( isset($HTTP_POST_VARS['smile_pak']) ) ? $HTTP_POST_VARS['smile_pak'] : $HTTP_GET_VARS['smile_pak'];

$smile_pak gets its value from either a post or from URL parameter, which is potentially tainted by the user, but line 52 ensures that the user has admin rights and in this way can be trusted.

In usercp_sendpasswd.php, line 32-33:

 $username = ( !empty($HTTP_POST_VARS['username']) ) ? trim(strip_tags($HTTP_POST_VARS['username'])) : ;
 $email = ( !empty($HTTP_POST_VARS['email']) ) ? trim(strip_tags(htmlspecialchars($HTTP_POST_VARS['email']))) : ;

The password reminder gets the user name and e-mail address from the user and sends the password by e-mail. This information can be trusted as we suppose that the user has exclusive access to her e-mails.

Verify that users can only access services for which they possess specific authorization.

Status: Pass

Since we were unable to settle on what's the difference between "function" and "service", we decided to consider these two to be the same thing. Users are authenticated by session management: the session id comes from a cookie so it could be tainted, but the database stores the user id and IP associated with the session so it cannot be used for remote hijacking. Also check the next item.

Verify that users can only access data for which they possess specific authorization.

Status: Pass

Authorization check is done in includes/auth.php. There is a huge "auth" function which returns what things a particular user can do with a particular forum (view, read, post, reply, edit etc.).

Verify that all access control decisions can be logged and all failed decisions are logged.

Status: Fail

The auth function doesn't do any logging. It simply returns a list of permissions and doesn't care what the caller wants to do with them.

In viewforum.php, there is a small part which calls auth and checks the result. Upon finding that the user doesn't have permission, it calls message_die, which outputs an elaborate error screen but also doesn't log anything. The same principle applies to the other files that use auth as well.

Verify that users can only access URLs for which they possess specific authorization.

Status: Pass

There are two types of URL that deserve mention. One are files for inclusion only, such as common.php and extension.inc. The files which are intended to be accessed from the outside start by defining a symbol IN_PHPBB. Include-only files start with a check for this symbol and exit if it's not defined, thus preventing further access. The other are the admin features which of course must be restricted to admins only. This is done using the "auth" function described above. It is called in an include file that is included in each of the admin pages and exits if the user is not admin.

Directory listing (in itself a dangerous feature) is avoided by including an index.php or index.htm file (even if blank) in every directory, as the default behaviour of the web servers is to display this file rather than the directory list. However, this also depends on the configuration of the web server.

Verify that access controls fail securely.

Status: Uncertain (depends on whether templates are injectable)

The results from the "auth" function are checked after the call and either the user is redirected to the login page (if not logged in) or given an error message via the message_die function. This function displays an elaborate error message, and this process involves template parsing using the eval (="evil") function. Therefore, an attacker capable of modifying the templates can perform code injection, although this is true for everything else, not just the access controls. If we disregard the fact that templates are used, we can say that the failure is secure and doesn't leak vital info.

Verify that the same access control rules implied by the presentation layer are enforced on the server side.

Status: Pass

This could be a problem if our application had access control in other layers (client side) with different rules. phpBB has access control rules only on the server side. We think it is not a problem because phpBB doesn't rely on client side access control anywhere. The access control rules on the server side are checked in the other parts of the project.

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

Status: Pass

The previous criterion (Verify that the same access control rules implied by the presentation layer are enforced on the server side.) is a subset of this. phpBB has all access controls on the server side.

Verify that there is a centralized mechanism (including libraries that call external authorization services) for protecting access to each type of protected resource.

Status: Fail

The only authentication feature is the "auth" function, which just returns a result regardless of what it will be used for. However, access to the actual resources are not checked. Malicious code can simply access the data from the database without calling the auth function. We think that this requirement is superfluous as the current structure (auth function, admin check etc.) is centralized enough for its purpose, but changing it to match the wording of the requirement would require much tighter coupling of the auth and database-related functions which could be labour-intensive if not impossible. Spending that labour to prevent code injection (the only thing that causes trouble with this requirement) is more useful spending of the resources.

Verify that limitations on input and access imposed by the business on the application (such as daily transaction limits or sequencing of tasks) cannot be bypassed.

Status: Uncertain

We have only a few ideas about what can be a business, what kind of limits it can have in phpBB. We think these businesses can be for example: the number of messages that we can send after each other; we can't send a message into a topic that doesn't exist.

We don't think that there is a limit for the number of messages that we can send after each other. We checked the second problem: "posting.php" checks if the topic_id or the forum_id exists or not, and it also checks the result of the query, so it can't be a problem.

So we don't know how to find other limitations.

Verify that all user and data attributes and policy information used by access controls cannot be manipulated by end users unless specifically authorized.

Status: Pass

We think these changes are only possible in the admin pages. Since the admin pages check for the presence of the admin rights, we consider this requirement to be fulfilled.

Conclusion

• It should not use templates with the "eval" function, but simpler templates with text substitution and maybe an ad-hoc interpreted mini-language to stand in for any features that lead to the choice of using "eval" in the first place
• It should log failed authentication attempts. Since the controls to access restricted functions don't even appear if the user doesn't have permission, a failed authentication attempt could be a fair warning sign of hacking.

• Otherwise access control is quite good, as long as the pages are not vulnerable to XSS or SQL injection, but that's not our task to check.