SoftwareSecurity2012/Group 6/Verdict

Composite of SQL Statements

The SQL statements which are created by FluxBB code consist of fixed parts and variable parts. These variable parts are program variables which depend on the user input and are appended (in)to the fixed parts of the string. Take for example the query

 $db->query('INSERT INTO '.$db->prefix.'reports (post_id, topic_id, forum_id, reported_by, created, message)  
 VALUES('.$post_id.', '.$topic_id.', '.$forum_id.', '.$pun_user['id'].', '.time().', \.$db->escape($reason).'\')' )  

In this case, the query method of the global db object is provided a string. "INSERT INTO", for example, is a fixed part of the string, whereas $post_id and $db->prefix are variables parts which are appended to it. By influencing the values of these variable parts, an attacker might be able to execute an SQL injection if these variable parts are not escaped properly.

While checking the files for such SQL queries, we came across three different basic cases for the variable parts: Either the variable part is real user input (e.g. _POST[…] or _GET[…]), it is local internal input (e.g. post_id) or it is global internal input (e.g. db->prefix, pun_user array). In the following we will describe these three cases in somewhat more detail:

For real user inputs, for example $reason in the above mentioned query, the content is always escaped using the escape function of the global db object. For a detailed description of this function, see the explanation on the db layer. All occurrences of such real user inputs that we found were properly escaped using that function, thus we can conclude that no SQL injections are possible in this case. (Erik:Still, it would have been better if they used parameterized queries of course. )

For local internal inputs, we discovered that they always describe integer values, like $post_id in the above mentioned query. All occurrences of those integer values are checked using the intval function of php. This function tries to convert the argument it is provided into an integer. If it fails, it returns 0. By tampering with such a local internal input, the attacker can thus only change the integer value. However, it is impossible to execute an SQL query.

For global internal inputs, we distinguish two cases. On the one hand, the $db->prefix field is used very often. It describes a fixed prefix for each table in a database and makes it possible to run multiple instances of FluxBB within the same database. This prefix is only set by the administrator during the installation process. However, this very administrator already has the db credentials at his disposal and would not extend his rights by executing an SQL injection via the prefix. In other words, we do not consider this administrator as a possible attacker. After the installation process, the prefix is read from the config.php file, but never changed again. Therefore, we consider this prefix safe regarding an SQL injection.

On the other hand, fields from the pun_user array are also used in queries very often. This array is created in the file include/common.php and can be accesses everywhere due to its global scope. It is filled by the function check_cookie() in include/functions.php with data that is directly taken from the database. Contents are all fields from the user and group tables, as well as the fields logged and idle from the online table (description of DB structure). The fields used from the online table are both integers. Those from the user and group tables are, however, integers or character strings. When they are used inside queries, those integer values are not escaped, but can be considered safe regarding SQL injections with the same argument as given above: they are after all integers inside the database. Considering character string fields in the pun_user array, we found that they are each time again properly escaped using the escape function of the global db object. Thus for the pun_user array fields, again, an SQL injection is rendered in our opinion highly unfeasible and we conclude the same for the whole global internal input case.

V6.3 Verify that output encoding /escaping controls encode all characters not known to be safe for the intended interpreter

The object $db, which is used to communicate with the database, is created depending on the database in use. For each supported database, a function escape() is provided escaping given input so that it is harmless for the SQL interpreter. This escape() function is used on all user input. Further details on the structure of the DBLayer can be found here.

Thus we conclude that this requirement is passed.

V6.4 Verify that all untrusted data that is output to SQL interpreters use parameterized interfaces, prepared statements, or are escaped properly

Neither for the real user inputs, nor the local internal inputs, nor the global internal inputs we were able to find data that is not properly escaped. Thus we conclude that the FluxBB developers took the threat of SQL injections seriously and provided protection against it in a structured way throughout the whole project by escaping the input. The developers also demonstrated consistency across all supported database platforms. However, neither prepared statements, nor parameterised interfaces are used.

We still conclude that this requirement is passed.

V6.9 Verify that for each type of output encoding/escaping performed by the application, there is a single security control for that type of output for the intended destination

This is a level 3 requirement but we offer some insight. By observing the architectural structure in the Database Layer and the abstraction above the layer, we can see that the programmers indeed used an architecture that eliminated all the possible escaping routes down to a single security chokepoint. A schematic representation is view-able below. All data has to pass through the escape() function, regardless of the underlying DB mechanisms. We consider this requirement as almost passed because of the architecture is widely used. We still note that it is not rigorously enforced, yet the PHP language does not include mechanisms to make only the abstraction layer visible. Extending the fluxBB project could be implemented with or without the abstraction.