SoftwareSecurity2014/Group 10/Reflection

What difficulties did you encounter, either with the code or the ASVS? Can you think of ways to reduce or avoid these difficulties?

Code

The Piwigo project does not appear to have some document available that describes their software architecture, or that gives a high-level overview of the code and thheir modules in some other way. We really only have a bunch of files through which we must search for keywords, or look at file names, to determine where some particular kind of functionality is present. For instance: in order to examine Piwigo's use of session management, we simply perform a search for the word 'session' in all source files and start analysing these files. When security issues that are discovered by Fortify are concerned, the tool's directions can simply be used to find the relevant source files.

Despite this, it was not that difficult to navigate the Piwigo source code because it is relatively straightforward and it could often be quickly derived what a certain piece of code was supposed to do. The amount of comments is rather low, though. Most functions do have Doxygen documentation, but the descriptions are often trivial. Parameter and return value types are noted, which is useful.

There are some files which do not contain functions but just directly execute code. This is bad practice by itself, but they also don't have any documentation on the locations where they could be included.

Occasionally, third-party modules (such as phpass and JShrink) are directly included among the other source files, rather than putting those in a seperate directory. These files tend to have a different style and use different conventions, which can sometimes be confusing.

In general, Piwigo would probably benefit from its source files clearly being divided among modules, which should be reflected by the directory structure. That would make a security analysis of the source code a lot easier. A document describing this structure and what can be found where would also be useful.

ASVS

The difference between some requirements, such as V5.2 and V5.6 are hard to notice, which has lead to some confusion while interpreting the requirements. This problem might be solved by adding some additional commentary about each requirement. Secondly, one of the difficulties is that ASVS provides standard verification (level 2) which is typically made for sensitive functions and procedural data in software. This could be solved by having Level 2 focus on both Application Security Auditing and Application Security Testing, instead of just the latter, as both features are equally important.

The ASVS was not that difficult to work with, although it would have been useful if the listed requirements would have been linked to a detailed description and guide of how to deal with them (e.g. by including this information or by referring to the corresponding OWASP Wiki pages).

Could the ASVS be clearer, more complete, or structured in a better way?

As said in the previous section, there should have been a link from each verification requirement to information about the issue, how to check for it and how to mitigate. (Erik: I see your point, and it's a valid one. It seems that the ASVS authors have tried to keep the document short and sweet, at the expense of being not very detailed. I can see the reasoning behind that - some of the documents in the OWASP wiki have become huge and bloated... )

Some of the requirements are a bit unclear: for example: "Verify that direct object references are protected, such that only authorized objects are accessible to each user.". What kind of objects are meant by this? Unclarity and misinterpretation of the requirements could also be prevented if they would simply refer to a more detailed document for every single requirement.

As part of both Cryptography and Authentication, the ASVS requires password hashes to be properly salted when created. However, this implies that a structure such as md5($salt . $password) (i.e. a regular cryptographic hash function directly applied to both password and salt, which will allow an attacker with a GPU or two and a bit of patience to brute-force most of the hashes in your database) is sufficient. This is definately not the case, and the OWASP should instead recommend using a proper key derivation function designed for storing passwords.

Some requirements seem to be rather similar: for instance "Verify that the session id is changed or cleared on logout." and "Verify that sessions are invalidated when the user logs out.".

Is splitting up the work as we did, with different groups looking at different security requirement, a sensible way to split the work? Or are there more practical ways?

While splitting up the work by security requirements is possible, it is not the most sensible. Invariably, what will happen is that each group will have to look at the entire code, and some groups might even be doing the same work due to partially overlapping requirements (Such as requirements dealing with input validation and output encoding, and requirements dealing with Authentication and Cryptography). Especially for larger projects, it might be faster to split up the work by having each group look at different parts of the code. Even though a group has to keep more requirements in mind, there is less overlap. This hopefully leads to a more efficient work load.

What could or should developers do to facilitate a security assessment? Do you think that experiencing security from the perspective of a reviewer rather than a developer has changed the way you would design or implement a web application yourself?

The most important thing is obviously documentation. A well-documented project, in which it is clear what each method is supposed to do, as well as a general overview of which code is responsible for what, is easiest to review. In addition, documentation may aid certain security tools in finding bugs. Secondly, it is helpful for developers to keep a set of general security requirements in the back of their head when creating software. However, we do not believe the perspective of a reviewer has changed the way we would design a web application. The importance of documentation should already be known to all software developers. In addition, the perspective of a reviewer is very close to that of a developer; the former should look at code and determine whether it is good code, and the latter should strife to create good code.

Any other interesting, frustating, boring, educational, etc. aspects of the whole experience.

• Commercial source code analysers, or at least Fortify, seem to be surprisingly effective at finding certain classes of vulnerabilities and also provide a lot of useful tools to analyse those. Of course, for many types of security issues static analysis is mostly useless and the process of identifying at what places in the code something interesting is going is still manual.
• We could find easily exploitable vulnerabilities within minutes after looking at Fortify's results. Assuming there are many open-source web application libraries for which this also holds (which is almost certainly the case), a hacker armed with a static analysis tool will have a very easy job exploiting many web applications.
• One would imagine that, by now, anyone designing a popular library used by a lot of web applications would understand not to mix user-supplied data with their SQL queries. Apperantly, this is still not the case, even though the problem is very easy to mitigate.
• RATS is surprisingly useless, even if the tool is just a glorified sequence of grep's we would have still expected some better results.
• It is much more fun to do analysis requirements the application completely fails at than noting that the requirements are simply met. (Erik: I guess you had some fun then :-) Did you try and contact the Piwigo people to tell them about your work? It seems they need to seriously improve their security... )