Detecting security flaws with FindBugs

This article was originally published on the Red Hat Customer Portal. The information may no longer be current.

Security response is largely a reactive process for handling problems that are already in software being used in production environments. The coordinated disclosure of vulnerability information attempts to protect software consumers from exposure to threats that are not yet public. However, it is much more desirable and cost effective to reduce the number of security issues that are introduced to the software during the development process. And while security training and awareness programs help raise the security mindset amongst developers the unfortunate reality of computer software is that mistakes are still made, and things are simply missed.

It is not simply a problem associated with developer competency, it is an issue that is bound to the inherent complexity in computer software. The number of CWE categories alone is enough to make your head swim, especially if you do not come from a security background. So supporting tools are a big part of helping an average software developer do their job better. One of the tools which I have personally been interested in lately is a pluggable static analysis framework for the Java language called FindBugs.

FindBugs does a pretty good job of finding security issues within compiled Java bytecode. Presently, there are detectors within the core and community lead projects that can detect issues such as SQL injection, XSS injection, and potential flaws that leave an application open to HTTP response splitting attacks. If most security issues can be mapped to a CWE, in theory they can also be traced back to a bad piece of code. By automating the detection of these weaknesses as they are reported, security response activities can feed directly into improving future development activities and theoretically reduce the incidence of the bug in future software releases. To get people interested and actively contributing to the FindBugs project I thought I would walk through a simple detector that can be used to find a common TLS anti-pattern.

Using TLS correctly and securely is often a deployment issue that is circumvented in code. The default JDK configuration will reject self-signed certificates when a client attempts to connect to a server. A common bad practice that people employ to bypass this roadblock is to implement a non-enforcing TrustManager class that effectively trusts everything. This custom TrustManager is then used to create a connection to a server resulting in a handshake where the authenticity of the server's certificate is never checked by the client. This compromises the legitimacy of the TLS connection, and is not the correct way to establish a working TLS connection using a self-signed certificate. The following is an example of the code we will be trying to detect:

[sourcecode language="java"]
class TrustAllTheThings implements X509TrustManager {
@Override
public void checkClientTrusted(final X509Certificate[] chain, final String authType) throws CertificateException {
// Do nothing
}
@Override
public void checkServerTrusted(final X509Certificate[] chain, final String authType) throws CertificateException {
// Do nothing
}
@Override
public X509Certificate[] getAcceptedIssuers() {
// Do nothing
return null;
}
}
[/sourcecode]

The heuristics that could be used to detect this anti-pattern are:

• The class implements the X509TrustManager interface.
• The checkServerTrusted method does not throw a CertificateException anywhere in the method body.
• The checkServerTrusted method does not call another X509TrustManager implementation.

If a class meets all of these conditions then there is a good chance the TrustManager is not being used correctly. The ultimate goal of any detector should be to detect this type of implementation problem without being overly intrusive and returning false negatives. To begin with, the core structure of the FindBugs plug-in needs to be established. A skeleton FindBugs project to be built using Maven might look like the following:

.
├── pom.xml
├── README.md
└── src
    ├── main
    │   ├── java
    │   │   └── com
    │   │       └── redhat
    │   │           └── pst
    │   │               └── findbugs
    │   │                   ├── classutils
    │   │                   │   └── Bytecode.java
    │   │                   └── detector
    │   │                       ├── EmptyHostnameVerifier.java
    │   │                       └── EmptyTrustManager.java
    │   └── resources
    │       ├── findbugs.xml
    │       └── messages.xml

The findbugs.xml file defines the detectors that are available within the plug-in and the bug patterns that are applicable to them. The messages.xml file contains informative descriptions of what the detector looks for, and reasons why matches are considered a bug. The detector we are writing to find this issue will need to inspect several aspects of a class file so the detector we use will be an extension of the BytecodeScanningDetector class. The visitor pattern is used by FindBugs to visit each class file within a JAR, incrementally increasing the granularity of the aspect being inspected. Each level of granularity can be overwritten to preserve state and build up complex usage patterns and detection rules.

The first heuristic that we are attempting to detect is that the class implements the X509TrustManager interface. To achieve this we override the visit methods for a Javaclass. At this level of granularity it is possible to determine if the class implements a TrustManager interface. This fact will be recorded and used later when inspecting the method implementation.

[sourcecode language="java"]
@Override
public void visit(JavaClass cls){
for (String implemented : cls.getInterfaceNames()) {
if (implemented.equals("javax.net.ssl.TrustManager") ||
implemented.equals("javax.net.ssl.X509TrustManager")){

            implementsTrustManager = true;
            javaclass = cls;
        }
    }
}

[/sourcecode]

The next two heuristics that were identified can be determined at the method level. Apache BCEL provides a mechanism to search a set of instructions for a particular sequence of operands. I have abstracted away the actual bytecode sequence for exceptions and invoking an interface in a utility class so the actual method visitor function is quite condensed.

[sourcecode language="java"]
@Override
public void visit(Method method){
if (implementsTrustManager){
if (method.getCode() == null) return;
String methodName = method.getName();
byte[] bytecode = method.getCode().getCode();
String certException = "java.security.cert.CertificateException";
String wrappedInterface = "javax.net.ssl.X509TrustManager";
ConstantPool cp = getConstantPool();
switch (methodName) {
case "checkServerTrusted":
case "checkClientTrusted":
// Check to see if any part of the code throws a
// certificate exception. Without throwing this exception
// it is unlikely that the code is correctly validating
// the server certificate correctly, or authenticating the
// client certificate respectively.
//
// Also handle the case if this class is wrapping
// another X509TrustManager instance.
if (! Bytecode.throwsException(bytecode, cp, certException) &&
! Bytecode.invokesInterface(bytecode, cp, wrappedInterface)){

                    // Make a record of the bug, and its location to be 
                    // reported when the caller invokes the visitAfter method.
                    bugs.put(methodName,
                        new BugInstance(this, BUG_PATTERN, BUG_PRIORITY)
                            .addClassAndMethod(javaclass, method));
                }
                break;
        }
    }
}

[/sourcecode]

That is pretty much all there is to it. Once built, the plug-in can be installed or loaded via the command line and run against a group of .jar files or integrated with your favorite IDE. Obviously this detection mechanism will not catch every instance of this class of bug, but it will help prevent the most common cases. It is my hope that the detection of programming errors that lead to Java CVE's can be automated in a similar way to this. If you're interested in creating your own FindBugs detector I've put the complete project on my github account: https://github.com/gcmurphy/pstfb.