SoftwareSecurity2013/Group 4/Reflection

SQL Output Validation

First of all it is important to notice what SQL Output Validation actually means. We have the opinion to not trust the database completely and see the SQL Output Validation as the validation process between the code and the database. It is of course not Input Validation, so the validation process needs to be done right before the call to the database.

Delimited what we check

We have delimited what we check because of time reasons given by the lecturer: it is impossible to check everything which has to do with the verification requirements of one group. We have chosen to only focus on the database layers and work from there into the complete framework. First of all because there are different database layers for different database systems (SQLite, MySQL, MySQLi, MySQL InnoDB, MySQLi InnoDB, PGSQL), we chose to focus one one of those and only look at the other ones if we suspect possible violations. The database layer at which we looked is MySQL InnoDB, because that one is most used as far as we could find. It also has the most similarities to MySQL, MySQLi, and MySQLi InnoDB. MySQLi is quite new and because of that it's most likely more secure, but as we are looking to violations, we chose to look at MySQL (with InnoDB as said before).

Our checking procedure

First of all we devided the work by all looking at around the same amount of lines of code, but always with complete functions in it. For every function we encountered, we checked every possible sequence of steps to search for possible violations of our applied ASVS verification requirements (6.2 to 6.4). With the information we got from looking at the function definition, we looked at all callstacks of the function - which are mostly in other classes used directly on the website, like userlist.php - also for the ASVS verification requirements. After having all that information and writing it on a first draft for the verdict on the verification requirements, we deliberated together if FluxBB failed, almost passed or passed the ASVS verification requirements.

Difficulty: too many calls to a function to check them all

Some functions had too many calls to it to check them all, for example the function query (almost 500 calls). It was impossible within the time limit to check them all, and also that wouldn't be that effective. We chose to focus on the most important calls, which was decided by the checker himself. Calls in db_update.php are far less important than admin_users.php, because the last one has direct interaction with an admin and the first one is almost completely hardcoded. (Erik:All the code is hardcoded:-) I guess you mean that in the first one all the parameters etc are hardcoded - ie there are no/few caller controlled parameters & arguments)

Our opinion about ASVS

The verification requirements of ASVS aren't that easy to separate completely. It is not entirely clear what they mean exactly. For example the first verification requirement that we checked (6.2): Verify that all output encoding/escaping controls are implemented on the server side. Do they mean that having output encoding/escaping on the client side is bad? But that doesn't make sense, because it's not bad if it is also done on the server side. Doing encoding/escaping twice is nothing to worry about if the encoding/escaping algorithm works completely (pseudocode: for all input A: encodePlusEscape(A) == encodePlusEscape(encodePlusEscape(A))). We decided to interpret as that it's not bad to have encoding/escaping done on the client side, but that it needs to be done on the server side as well. (Erik:I agree with your analysis that server side controls are not bad if these checks are also done client side, but I guess that is what the AVSV means when it says checks should be done client side)

ASVS could be much clearer if it came with some border examples which violate and does not violate the verification requirement. We missed those examples and therefore needed to decide by ourself as objective as possible when a verification requirement is violated.

Splitting up the work between groups

The way the work is splitted is in our opinion very good. We could focus ourselves on one thing and only one thing. When you need to focus on multiple things, you have the tendency to target on one of them at a time and forget about the others for a moment. The dangerous part of that is that you could easily read over a possible violation because you focus on another thing at that moment. The splitting done by verification requirements is also good because you always need to look at multiple classes for a complete view of the security of the framework, even if the work was splitted per class. For example: when you encounter a possible violation and aren't allowed to look for call stacks, it's very hard to decide if the violation is real and how bad it is.

Changes in our thoughts about security

We looked at code of others and searched for possible security violations. We found that FluxBB doesn't have a good two layer security based on two choke points. On choke point for input validation and one for output validation. FluxBB lacks of good choke points which made it harder to check the verification requirements, because we needed to be creative to look for the root cause of problems. Because of this, we will now more look at the bigger picture when we write a program. We will be more clear and not only for ourselves, but also for others if they need to change or verificate our code. Writing good understandable comments is important for someone else to understand your code

(Erik: You did say that there is a special database layer to support different types of databases. This would be a natural place for a chokepoint to do output validation, right? Suprising that they don't do the output validation there is a centralised chokepoint.

Insight into faults of others

By looking at how others write a program, we now see better how mistakes happen. Mistakes which violate security requirements but also mistakes which lead to bugs. That is positive for our programming skills.