SoftwareSecurity2014/Group 11/Verdict

Uit Werkplaats
Ga naar: navigatie, zoeken

Verdict for level 2B verification requirements

Labour division

Mark: V4.1, V4.2, V4.3, V4.4, V9.1
Marta: V4.5, V4.6, V4.7, V4.8, V9.2
Tomas: V4.9, V4.10, V4.11, V4.12, V9.4
Kevin: V4.13, V4.14, V4.15, V9.5

V4 - Access Control

# Summary State
V4.1 Verify that users can only access protected functions for which they possess specific authorization. Probably OK
V4.2 Verify that users can only access URLs for which they possess specific authorization. Pass
V4.3 Verify that users can only access data files for which they possess specific authorization. Probably OK
V4.4 Verify that direct object references are protected, such that only authorized objects are accessible to each user. Probably OK
V4.5 Verify that directory browsing is disabled unless deliberately desired. Probably OK
V4.6 Verify that users can only access services for which they possess specific authorization. Pass
V4.7 Verify that users can only access data for which they possess specific authorization. Pass
V4.8 Verify that access controls fail securely. pass
V4.9 Verify that the same access control rules implied by the presentation layer are enforced on the server side. don't know
V4.10 Verify that all user and data attributes and policy information used by access controls cannot be manipulated by end users unless specifically authorized. don't know
V4.11 Verify that all access controls are enforced on the server side. Probably OK
V4.12 Verify that there is a centralized mechanism (including libraries that call external authorization services) for protecting access to each type of protected resource. Probably OK
V4.13 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. pass
V4.14 Verify that all access control decisions can be logged and all failed decisions are logged. fail
V4.15 Verify that all code implementing or using access controls is not affected by any malicious code. don't know

Requirements

V4.1

Specific parts of Wordpress must be protected so only authenticated users with the right access level can access them. There is no means for an automated tool to figure out which part requires which authenticated and what authorization for access, so this has to be figured out manually.

The Wordpress application is basically split in two sections, the public part and the admin part. The public part of the site is made accessible without requiring specific authorization, apart from exceptional functionality that is explicitly protected by requiring an authenticated and active user (i.e. in /wp-comments-post.php). The administrative section of the site is different. All the URLs in the /wp-admin URL-context may only be accessed by authenticated and authorized users. It is verified that no functionality from the admin part is available from URLs in the public part of the site.

Still, Fortify reports about certain situations (“Dangerous File Inclusions”) where file inclusions might be vulnerable. These cases report about places where tricky things might be going on, or where Fortify can not adequate assert that proper input validation took place. When looking more closely, it seems that Fortify is quite strict in accepting that validation has taken place, because validation does happen. It seems safe to say that the dangerous file inclusions pose no obvious vulnerability to include admin functionality inside the public part.

So the public part is accessible without access restrictions, apart from specific functions (post a comment). This function includes checking whether a user is logged in, and whether the logged in account is active. So the public part passes on both URL- and protected funcion access restrictions.

The admin part on the other hand is a lot more complex. Each Wordpress-user is assigned one or more roles, and each role covers a set of permissions. Wordpress calls these ‘capabilities’. When a user logs in, its roles (and capabilities) are applied to the security context of the session. So there is a concept of a unified security context, and on the other hand there is a set of functions that can query this security context, in particular there is a “current_user_can()” function that tests for the authorization of a capability.

When a user accesses a URL from the admin part of the site, Wordpress initializes its runtime context (including security context), immediately followed by a check (“auth_redirect()”, calling “wp_get_current_user()”) whether the session has the security context of an authenticated user. This enforces that only authenticated users can access the admin part of the site. (Note: the session is based on a cookie, that is managed by Wordpress. In this analysis, we have no reason to believe that the session can be easily hijacked, so we trust the session.)

So it looks like there is a sinkhole that enforces authenticated user access; therefore the URL protection requirement for the admin part is also met.

Because there is no security requirements overview that actually defines the expected access control here, it is impossible to do a comprehensive check to see whether it matches the specification. (Erik:Good point.That is typically a problem when looking into V4.)) When making manual inspections, it seems that there are strategically placed “current_user_can()” calls, to make explicit checks before actually performing the requested functionality. Note that this is always done in a place in the code where you would expect it (deep inside nested function calls and if-statements), but there are a lot of checks in place. While it would take a lot of time to get an overview of all the capabilities and where they should be enforced, the global impression is that this requirement is probably OK.

V4.2

See V4.1.

V4.3

Access to data files might or might not be possible, this mainly depends on the configuration of the application server Wordpress is installed on. When dependent script files are executed directly (i.e. by making a request to https://hostname/wp-includes/atomlib.php), these scripts do not do any processing (i.e. they only define functions in the script context) and generate no output. So while this requirement actually doesn’t pass from an application perspective (files are directly referenceable), the design of the application protects itself from being vulnerable when the application server provides no protection.

V4.4

In the public part of Wordpress, direct reference of objects (i.e. blog-posts) is not really an issue, as there is no access restriction based on who is logged in or not: you either have the right to access posts or not. On the other hand, the admin part of Wordpress is another story. For example when looking at the functionality that manages posts and comments, the first line of defense is that it requires an authenticated user to be able to get to this functionality (see requirement 4.1). The second level of access control here, is that capabilities are checked for the provided blog post. So when you would manually change the object reference (change the ID in the URL), you can either end up with operating on a blog post that you do not have access to (access is rejected), or on another blog post that is also under the control of the user. So there is the second line of defense: no access outside your own objects.

Furthermore, when looking at the post/delete function, there is a third line of defense in the form of the prevention for Cross Site Request Forgery. A nonce is included in the calling page to prevent (external) references to perform an operation on behalf of the user.

Another look was given to the "redirect_to" parameter, that is part of wp-login's functionality. Since there is a common phishing related vulnerability based on open redirectors, this could be a potential problem. Wordpress actually has an active defense against that as well, by validating the value of the redirect_to parameter, based on highly detailed validation criteria (must be valid URL, must be a valid host, etc. tested through the 'wp_safe_redirect()' function).

There has been a manual inspection on different parts of Wordpress (not every operation was checked), where all three mechanisms were actually applied. So this requirement is probably OK.

V4.5

We have in nav-menu.php

function wp_nav_menu_item_taxonomy_meta_box( $object, $taxonomy ) {

       global $nav_menu_selected_id;
       $taxonomy_name = $taxonomy['args']->name;
       // paginate browsing for large numbers of objects
       $per_page = 50;
       $pagenum = isset( $_REQUEST[$taxonomy_name . '-tab'] ) && isset( $_REQUEST['paged'] ) ? absint( $_REQUEST['paged'] ) : 1;
       $offset = 0 < $pagenum ? $per_page * ( $pagenum - 1 ) : 0;


that is the code for browsing pages.

V4.6

some automated cracker scripts (or worms) intrude in services like WordPress. By default PHP enables running commands via shell with PHP functions like exec();, shell_exec(); , system();. but(Erik:I don't understand the use of 'but' here.) in wordpress we use that functions and we don't disable them.

For exampe, shell_exec() is used in

$data = shell_exec($cmd);

and them the data is used for

               $dataArr = preg_split("/\n/", $data, -1, PREG_SPLIT_NO_EMPTY);

spliting this data in different strings with the pattern "/\n/"

V4.7

See 4.6 (Erik: Given that you join them in the discussion here: Do you think the distinction between 4.6 and 4.7 is a bit silly in the ASVS? Idem for V4.1 and V4.2? )

V4.8

The access control internally uses several caches to increase decision times of access control. In most cases, all caches access control needed are invalidated immediately when a change is made in the access control configuration, how to assign roles to a user.


The main access controls depend on session cookies to authenticate a user and give a user authorization to access their specific data. Cookies and PHP $_SESSION variables are used by many plugins and themes for WordPress.

When a visitor is served a cached version of the site, then the PHP code is not executed. This means that any PHP code that looks for cookies or handles session variables will not work, simply because it is not running.

There are certain special cases, where caching is disabled. One such case is when the website visitor logs into WordPress. WordPress sets cookies related to that login, and our system is set up to recognize those WordPress login cookies. When those cookies are found and are valid, then page caching is disabled.

V4.9

Access control enforced in presentation layer does not mean that attacker cannot access private information. Automatic tools cannot really check if presentation layer access control is enforced server side, so it can only be done manually. After analysis some specific parts of the code we couldn't find anywhere implemented presentation layer access control just on its own. As a side note access control must be performed in the business layer, not only the presentation layer.

Cosidering such a big amount of code, we couldn't be assured that lone presentation layer access control is nowhere implemented.

V4.10

It's not really viable for us to verify that all user and data and policy information cannot be manipulated by unauthorized user. To check if user is logged in, is_user_logged_in() function is used, then function wp_get_current_user() (located in wp-includes/pluggable.php) is used to detect the current user and hence assign him correct access restrictions. Each pre-defined user (Super Admin, Administrator, Editor, Author, Contributor, Subscriber) is allowed to perform a default set of tasks (Capabilities) and nothing more. Everyone with lower powers than Administrator can only publish, manage, write or read posts, but not manipulate unrelated information.

For Administrator (or Super Admin) actions, check_admin_referer() (located in wp-includes/pluggable.php) function is used, which tests either if the current request carries a valid nonce, or if the current request was referred from an administration screen; depending on whether the $action argument is given (which is prefered), or not, respectively.

V4.11

To verify this requirement it's important to check if non-server (client) side (which is un-trusted) is a non deciding factor in providing access to important functionality or sensitive information. Client-side checks for authentication can be easily bypassed, allowing clients to escalate their access levels and perform unintended actions.

Some important things to check for:

  • Keep user identity verification in session
  • Load entitlements server side from trusted sources
  • Force authorization checks on ALL requests

After reviewing some important Wordpress source code (wp-login.php, wp-config.php) we couldn't find any important authorization or authentication procedures not performed on server side.

V4.12

Wordpress uses wp-config.php (located in root directory) file which, among other things, contains different authorization and authentication attributes, such as AUTH_KEY, SECURE_AUTH_KEY, LOGGED_IN_KEY, NONCE_KEY, AUTH_SALT,SECURE_AUTH_SALT, LOGGED_IN_SALT, NONCE_SALT. Note that all 4 secret keys are required for enhanced security, while 4 salts are optional (wordpress can generate them itself if not provided).

For secured logins via SSL, FORCE_SSL_LOGIN (located in /wp-includes/default-constants.php) function can be used to sent encrypted passwords.

define( 'FORCE_SSL_LOGIN', true );

To secure both logins and cookies with SSL, FORCE_SSL_ADMIN function can be used (this is optional but most secure).

define( 'FORCE_SSL_ADMIN', true );

Some other possible security measures include:

  • Blocking external URL requests (WP_HTTP_BLOCK_EXTERNAL)
  • Disable plugin and theme updates (DISALLOW_FILE_MODS)
  • Disable plugin and theme editor (DISALLOW_FILE_EDIT) - to prevent aggressive users editing possible sensitive information
  • Disabling WordPress auto updates (AUTOMATIC_UPDATER_DISABLED)

Note that we are not exactly sure that in fact each type of protected resource is protected using centralized mechanism.

V4.13

This one is a little hard to define, but we think we should focus on two parts of Wordpress namely, xmlrpc and WP-Cron. There are no real limiters in wordpress they are always created through plugins and we will not dive into plugin reviews now.

XML-RPC

Wordpress supports remote procedure calls to call API related things from outside the application. Everything for this is defined in "class-wp-xmlrpc-server.php" and when looking for this file in Fortify there are only two catogries of issues.

Password in Comment (35): This are all false positive because of the documentation in the source they say something like $password is password (javadoc) thus triggering all these false positives.
Empty Password (1): They create an array (inside _insert_post helper function) with 'post_password' => ' '. However this later on gets merged with the real post data and afterwards its also gets checked for the correct password.

When delving further in the checks if users can access the RPC. In every RPC function there is a piece of code like:

if ( !$user = $this->login($username, $password) )
	return $this->error;
...
if ( !current_user_can(X))
	return new IXR_Error( 401, __( 'x' ) );

We have checked 1/4 of all the RPC functions and they all had the login check and one or more current_user_can checks. So we think XML-RPC is safe in an access point of view.

WP-Cron

This is there system for doing tasks just like unix cron. Well not really, because they run the task when ever someone access the site and a task has to be done (like posting an article at X). Everything for this system is defined in cron.php. Fortify gives us only one issue, namely:

Weak Cryptographic Hash (8): While its true its weak (they use md5) they use if for hashing simple keys for storage and retrieving. I myself often also use this technique to create a small key from binary data and there is nothing wrong with this.

Also the whole cron system is locked down behind the admin interface and there are other requirements for that part. So we think its ok.

V4.14

Wordpress has no real logging capabilities. What they do is they redirect all PHP errors (they can be real PHPerrors, or exceptions thrown by logic) and let PHP save them to a file. So with this you can already say this requirements falls short. However, if one looks at the function current_user_can then you can see that they do not even log or throw errors. It just gives true or false.

But when you look at the methods that call current_user_can they often throw some sort of exception but most of those exceptions are nicely handled and thus not logged inside the log. So this requirements fails.

We do want to note that there is an excellent plugin that makes wordpress pass this requirement

V4.15

This is a gigantic task to check this because there is no nice uniformly system for handling access control (in other words, every function checks access controls for themselves). And as this is an open source project it is impossible to check for this because there is a constant flow of new code from maintainers. (Erik: Note that V4.15 is actually out of scope for a level II evaluation - precisely for the reason you state. )


Everything for access control is managed with Roles and Capabilities. Thus users have roles and those roles have capabilities giving you "access" to something. This is checked by the functions "current_user_can(), user_can()".

The correct way to tackle this requirement would be to find all calls to user_can and current_user_can. After this you should analyse the code directly related to the "user_can" part. But even if you would do this then there is no guarantee that code more upstream is not setting up things wrongly, or even code downstream.

We already checked 1/4 of the XMLRPC code responsible for this (for requirement V4.13), and found no malicious code (on the looks of it).

In wordpress there are 742 calls to eater current_user_can or user_can (or function declarations). We decided to do check randomly 7 files (1%) with these calls. And found no malicious code. However to say that we pass this requirement is far to strong. There has to be done a lot more checks. But because the amount of time this takes would get out of hand we decided to leave it like this.

wp-admin\admin.php: nothing malicious found

  • @122: There is a check if you have the role 'manage_options', if you have the MEMORY_LIMIT PHP setting is increased (or decreased). This has no other implications.
  • @255: When the GET action is 'import', there is a check if you have the capabilities for 'import'. If you do not then it dies with wp_die.

wp-includes\class-wp-customize-manager.php: nothing malicious found

  • setup_theme(): Check if you have 'edit_theme_options' if you dont it dies with wp_die. Nothing malicious found.
  • setup_theme(): Check if you have 'switch_themes' if you dont then it does (when you are switching themes). Nothing malicious found.

wp-includes\comment.php: nothing malicious found

  • check_comment_flood_db(): Function to throttle spammers. If you have the 'manage_options' right then the throttling is disabled. This is of course by design.

wp-admin\comment.php: nothing malicious found

  • @70: There is a check for 'edit_comment', if you do not have this right then there is a die with comment_footer_die.
  • @96: There is a check for 'edit_comment', if you do not have correct right you get a hard redirect and a php die.
  • @222: There is a check for 'edit_comment', if you do not have the correct right then there is a die with comment_footer_die.

wp-includes\default-widgets.php: nothing malicious found

  • update(): There is a check for 'unfiltered_html', if that right is enabled then the text is given back without removing html slashes. Else html is escaped.
  • wp_widget_rss_out(): There is a check for 'manage_options' that when you are an admin and you have this roll there is real debug information, else nothing is printed.

wp-includes\pluggable.php: nothing malicious found

  • wp_notify_postauthor(): There is a check for email notifications on a post. If you are can no longer read the post then you do not get mails.
  • wp_notify_postauthor(): If you have the capabilities to edit the comment 'edit_comment' then you get extra links to directly trash or delete or spam the comment.
  • wp_notify_moderator(): Checks if the author can modify the comment 'edit_comment' and if so send a mail to the author.

wp-admin\network\site-info.php: nothing malicious found

  • @16: Checks if you have the right 'manage_sites' if not it just dies with a message, else it continues.

V9 - Data Protection

# Summary State
V9.1 Verify that all forms containing sensitive information have disabled client side caching, including autocomplete features. Should Improve
V9.2 Verify that the list of sensitive data processed by this application is identified, and that there is an explicit policy for how access to this data must be controlled, and when this data must be encrypted (both at rest and in transit). Verify that this policy is properly enforced. Don't know (?)
V9.4 Verify that all cached or temporary copies of sensitive data sent to the client are protected from unauthorized access or purged/invalidated after the authorized user accesses the sensitive data (e.g., the proper no-cache and no-store Cache-Control headers are set). pass
V9.5 Verify that all cached or temporary copies of sensitive data stored on the server are protected from unauthorized access or purged/invalidated after the authorized user accesses the sensitive data. fail

Requirements

V9.1

The forms that contain sensitive information in Wordpress, are all related to asking the username and password. This means that the login form is critical, and the form where a user can manage its profile or its password.

Authenticating a user happens at one place, in the wp-login.php script. This page explicitly sets HTTP Response headers to disable cache (“Cache-Control” and “Pragma no-cache” headers are sent). The password field has the ‘password’ type. But there is no more protection than that, so no explicit disabling of auto completion.

When managing a profile, the password fields are protected by the attribute autocomplete="off", on top of the no-cache strategy and ‘password’-type input fields. So this might be an improvement on the wp-login page.

This criteria party meets the requirement, so the verdict is Should Improve.

Extra thoughts: based on this analysis, when authentication and password management is removed from Wordpress (which can be done by using federated authentication) this requirement is met a lot more easily.

V9.2

(see better)

The function that gets user and data information by login , get_user_by():

$site_user = get_user_by( 'email', $email );
       if ( ! is_email( $email ) )
               $errors->add( 'invalid_email', __( 'You must provide a valid e-mail address.' ) );
       if ( $errors->get_error_code() )
               return $errors;

identify the sensitive data (login information).

V9.4

The only really sensitive data in Wordpress is login information (usernames, passwords), but they are sent to the server, not the client, so it is not an issue.

V9.5

Because wordpress is written in PHP you can be very quick about this one. There is a file in the tmp folder of many PHP installations that keep track of session identifiers. With that identifier you have all the rights of that session (if its logged in you have many, if its an admin then you have everything). However many systems implement session cleaning different but most of the time it is as simple as. Sometimes check if the session is not used for X amount of time. If so throw it away. Because this simple fact of how PHP (most of the time) works. We can give this requirement a fail.

Extra consideration: The PHP environment allows configuration to decide where session data is stored (filesystem, database, custom). Wordpress by itself doesn't seem to use temporary files with sensitive information, so on that basis, the requirement can be considered a pass.