SoftwareSecurity2012/Group 8/Verdict

V5 - Input Validation Verification Requirements

Overview

Security requirement	Level 1B	Level 2B	Pass / Fail	Description
V5.1	✓	✓	Pass
V5.2	✓	✓	Pass
V5.3	-	✓	Pass
V5.4	-	✓	Fail
V5.5	-	✓	Pass
V5.6	-	✓	Pass
V5.7	-	✓	Fail
V5.8	-	-	-	This is a level 3 requirement.
V5.9	-	-	-	This is a level 4 requirement.

All results shown below have been achieved through manual auditing, unfortunately the tools their false positive rate was too high to be of any use.

V5.1 - Verify that the runtime environment is not susceptible to buffer overflows, or that security controls prevent buffer overflows

PHP is not susceptible to buffer overflows by design. You must however make sure to be running the current version as there have been bugs which revealed internal pointers such as MOPB-35-2007. This is a patch management issue however and not something related to FluxBB.

Auditing PHP for bugs such as the one described above is obviously out of the scope of this project.

---

V5.2 - Verify that a positive validation pattern is defined and applied to all input

PHP applications generally take input from the world via the arrays: $_GET, $_POST, $_COOKIE, and $_FILE. FluxBB ammends this list by also peeking into the $_SERVER array to get the remote host of a connecting user. FluxBB defines and makes use of a generic function forum_remove_bad_characters() to sanitize these arrays, which is directly called through include/common.php, a file that is included in every 'interesting' or 'dangerous' script file in the package. This effectively reduces all UTF8 input to ASCII input, useful because confusion between UTF8/ASCII input types is often used to bypass input filters in other application domains.

The application comes with its own SQL abstraction layer, allowing it to use the same SQL queries to access various databases. As far as we were able to determine, all database calls that make use of user input call a database-library-specific escape() method in the database abstraction class.

In order to validate a certain pattern the preg_match() function is used as a blacklist (Erik: note that preg_math is not used as a blacklist, but that the arguments of preg_match are the blacklist, or the white list, depending on whether preg_match is used to detect out the bad stuff or the good stuff) to verify if certain input is incorrect. In several occasions it checked whether an IP-address is of the correct format and also if it does not contain a range such that it blocks out an administrator. In the same way email addresses, usernames and search-terms are checked for bad patterns, for example:

(Erik: By talking of `positive validation patterns' in V5.2, the AVSV indicates that white-listing rather than black-listing should be used. The wording you use above (eg. `blacklisting', `bad patterns') suggests that fluxbb might be using blacklists, which might (should?) result in a failure for this security requirement... )

   // Validate username and passwords
   if (pun_strlen($username) < 2)
       $alerts[] = $lang_install['Username 1'];
   else if (pun_strlen($username) > 25) // This usually doesn't happen since the form element only accepts 25 characters
       $alerts[] = $lang_install['Username 2'];
   else if (!strcasecmp($username, 'Guest'))
       $alerts[] = $lang_install['Username 3'];
   else if (preg_match('%[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}%', $username) || preg_match('%((([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}((\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b)\.){3}(\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b))|(([0-9A-Fa-f]{1,4}:){0,5}:((\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b)\.){3}(\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b))|(::([0-9A-Fa-f]{1,4}:){0,5}((\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b)\.){3}(\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:))%', $username))
       $alerts[] = $lang_install['Username 4'];
   else if ((strpos($username, '[') !== false || strpos($username, ']') !== false) && strpos($username, '\) !== false && strpos($username, '"') !== false)
       $alerts[] = $lang_install['Username 5'];
   else if (preg_match('%(?:\[/?(?:b|u|i|h|colou?r|quote|code|img|url|email|list)\]|\[(?:code|quote|list)=)%i', $username))
       $alerts[] = $lang_install['Username 6'];

and

   // If it's a regular search (keywords and/or author)
   else if ($action == 'search')
   {
       $keywords = (isset($_GET['keywords'])) ? utf8_strtolower(pun_trim($_GET['keywords'])) : null;
       $author = (isset($_GET['author'])) ? utf8_strtolower(pun_trim($_GET['author'])) : null;
       if (preg_match('%^[\*\%]+$%', $keywords) || (pun_strlen(str_replace(array('*', '%'), , $keywords)) < PUN_SEARCH_MIN_WORD && !is_cjk($keywords)))
           $keywords = ;
       if (preg_match('%^[\*\%]+$%', $author) || pun_strlen(str_replace(array('*', '%'), , $author)) < 2)
           $author = ;

---

(Erik: I must say that these very complex arguments of preg_match above scare me a bit :-) I do hope that, say for user names, there is only one central place where they do the input validation so that these complex expressions occur only once, and are not copied and pasted in various places in the code. But that is covered by V5.6, I now realise.)

V5.3 - Verify that all input validation failures result in input rejection or input sanitization

All input is filtered using forum_remove_bad_characters() and stripslashes_array() which remove 'bad' UTF-8 characters and slashes. The former function will throw an error when the validation fails whilst the latter triest to clean the input by removing slashes.

Input then continues on to the function where it is actually used. Variables in $_POST, $_GET, $_COOKIE and $_REQUEST are validated using regexes. Input that doesn't validate correctly is simply ignored when possible and will cause an error to be thrown otherwise. Input in the $_FILES variable is also checked using regexes and other validation functions which throw errors if the input doesn't match the expected format. It appears that all such situations are handled sufficiently.

---

V5.4 - Verify that a character set, such as UTF-8, is specified for all sources of input

This requirement is not really feasible in PHP, though there are workarounds. The database is configured to store all input as UTF-8. The webserver can be configured to use UTF-8 as the default character set for PHP and for its webpages. Each webpage that FluxBB generates contains the UTF-8 charset meta-header. Every input gets filtered using forum_remove_bad_characters() to remove any bad UTF-8 characters, and data that is output to the webpage is also filtered using pun_htmlspecialchars() with UTF-8 as charset.

   //
   // Removes any "bad" characters (characters which mess with the display of a page, are invisible, etc) from user input
   //
   function forum_remove_bad_characters()
   {
       $_GET = remove_bad_characters($_GET);
       $_POST = remove_bad_characters($_POST);
       $_COOKIE = remove_bad_characters($_COOKIE);
       $_REQUEST = remove_bad_characters($_REQUEST);
   }

and

   //
   // Calls htmlspecialchars with a few options already set
   //
   function pun_htmlspecialchars($str)
   {
       return htmlspecialchars($str, ENT_QUOTES, 'UTF-8');
   }

There is also a conversion function in db_update.php that converts ISO-8859-1 to UTF-8, but is ONLY used for database updates when upgrading the forum version.

   if ($old_charset != 'UTF-8' && !seems_utf8($str))
   {
       if (function_exists('iconv'))
           $str = iconv($old_charset == 'ISO-8859-1' ? 'WINDOWS-1252' : 'ISO-8859-1', 'UTF-8', $str);
       else if (function_exists('mb_convert_encoding'))
           $str = mb_convert_encoding($str, 'UTF-8', $old_charset == 'ISO-8859-1' ? 'WINDOWS-1252' : 'ISO-8859-1');
       else if ($old_charset == 'ISO-8859-1')
           $str = utf8_encode($str);
   }

However, there is no specification or rule that enforces the actual input to use the UTF8 character set in FluxBB.

This can be accomplished by forcing the data type. If an attacker can force a specific character set, then they might be able to bypass your data validation filters. An example of forcing UTF-8, and silently ignore/drop any characters that can’t be represented by that character set:

   // Supply the user supplied data to the forceUTF function.
   // This function will use the iconv PHP function to convert
   // the data to UTF-8, and drop any characters that can't
   // be converted into the charset.
   function forceUTF($data) {
       $utfEncoded = iconv("UTF-8", "UTF-8//IGNORE", $data);
       return $utfEncoded;
   }

---

V5.5 - Verify that all input validation is performed on the server side

Since no input validation is performed on the client side we can trivially conclude that all validation is performed on the server side if it is performed at all. To verify this we've checked all JavaScript files for functions validating input, common.js and include/minmax.js to be exact. Secondly we've checked all inline JavaScript, which there was very little of, for similar functions.

---

V5.6 - Verify that a single input validation control is used by the application for each type of data that is accepted

All text input is validated as indicated by 5.2, (Erik: The discussion of V5.2 mentions that there are some centralised validation functions, eg forum_remove_bad_characters() and for treating library-specific SQL filtering, but still leaves it open if there is, say, one function that does the validation of user names that is called in all places. Eg. the code sample shown checks for usernames and passwords, which suggests there might be other pieces of code which check for usernames, without passwords. So it would be good to state clearly that for each type of data there is a single validation control.) however the application also allows binary file uploads to set a profile picture. For this functionality the accompanying mime-type of the uploaded file (that is sent through HTTP in tandem with the file data), is first checked against a whitelist which only allows image mime-types. Because this information can be falsified the uploaded file is subsequently moved to a temporary location and its mime-type is checked via the getimagesize() function, which returns mime-types for image files. The type information acquired through this function call is used to rename the file to its proper extension before it is moved to the assets directory within the webserver root. This excludes the possibility of files which have an executable type (PHP, HTML containing Javascript) from being uploaded to the server.

(Erik: The way I understand the AVSV, the discussion about checking these image files belongs in V5.2. V5.2 is about whether these images are filtered properly, V5.6 is about whether there is a single control that does this for all images, ie. about checking that there are not two places in the code where images (or usernames, or other types of user input) are uploaded, each with their own input validation checks.)

---

V5.7 - Verify that all input validation failures are logged

FluxBB does not perform any logging at all thus no input validation failures are logged.

---

V5.8 - Verify that all input data is canonicalized for all downstream decoders or interpreters prior to validation

This is a level 3 requirement and therefore outside the scope of this project.

---

V5.9 - Verify that all input validation controls are not affected by any malicious code

This is a level 4 requirement and therefore outside of the scope of this project.

---