Focused code reviews - a followup

I promised something more technical than book reviews, so here it goes.

Earlier I posted about how to limit the amount of code for day-to-day security reviews if the code base is huge. I took Confluence (I work for Atlassian) as an example. The application uses Webworks 2, and other frameworks. Source code is not entirely free or public, but you can get it if you have almost any kind of Confluence license. I will keep some details out of this example.

Here are some things to trigger security reviews on this codebase.

Java generalities

Monitor for these being added, but there is no urgent need to review code if any of these get removed by developers. The list in this section is Java generic (and incomplete) and can be used for other apps, the other sections are more Confluence-specific. You might not need to trigger on all of these strings. You can also try structures from the IntelliJ searches from another blog entry.

Class.forName
ZipFile
Statement
Math.random
sendRedirect
"SELECT "
java.sql.Statement
java.sql.Connection
executeQuery
Runtime.
java.lang.Runtime
getRequestURI
java.sql
BeanUtils.setProp
java.lang.reflect

...

Sanitizers

Monitor for disappearance of any sanitisers from your code. There are legitimate reasons for this - for example a sanitiser in a view disappears but the corresponding model starts escaping or filtering data.

htmlEncode

...others skipped...

Filters


Being a Webwork2 webapp, Confluence utilises a number of filters and interceptors. You can get a list of filters your application uses with something like

grep -Rh --include=*.xml "<filter-name" . |sed -e 's/<filter-name>//'|sed -e 's/<\/filter-name>//'|sed -e 's/^[ \t]*//' |sort |uniq

Review the list and decide which ones have important security function. Monitor any change mentioning interceptors (both in web.xml files and for any change of their source)

HeaderSanitisingFilter
SecurityFilter
...
SafeParametersInterceptor
PermissionCheckInterceptor
...

Annotations

Some of these are generic, some are Confluence specific. One way of getting a list of all annotations is

grep -Rh --include=*.java "^\s\+@" . |sed -e 's/^[ \t]*//'  |sort |uniq

Example of what to monitor for:

@AnonymousAllowed	adding
@GET	adding
@POST	adding
@HttpMethodRequired	any change
@ParameterSafe	removal
@Path	adding
@RequireSecurityToken	removal
...

XML config files (new endpoints)

Action mapping etc - they introduce new URL endpoints. Monitor for adding, not removal.

"<action name" 
...

Other XML

Any change mentioning your filters or interceptors in web.xml, for example

<filter-name>header-sanitiser
<filter-name>request-param-cleaner
<filter-name>login

<interceptor-ref name="params"/>
<interceptor-ref name="permissions"/>
<interceptor-ref name="xsrfToken"/>

<interceptor-stack name 

...

Files and path

Look for any change in files used to implement crucial security features - login, session management, authorisation, sanitizers, CSRF protection and so on. 

confluence-core/confluence-webapp/src/main/webapp/WEB-INF/web.xml
confluence-core/confluence/src/etc/standalone/tomcat/web.xml
confluence-core/confluence/src/java/com/atlassian/confluence/security/login/*
confluence-core/confluence/src/java/com/atlassian/confluence/rpc/auth/*
confluence-core/confluence/src/java/com/atlassian/confluence/security/*
...

Monitoring for any web.xml changes is probably an overkill, you will catch interesting stuff with the items from other sections above).