A while ago a critical vulnerability in Spring AMQP was reported by Matthias Kaiser. This was caused by Spring AMQP performing unsafe deserialization on messages received from remote users. By default, Spring AMQP uses the SimpleMessageConverter class to convert messages that it receives into Java objects without any restriction on the type. This meant that, by sending a specially crafted message to the application, an attacker could force it to deserialize any classes that happen to be on the classpath and use it to perform remote code execution to take over the server.

The Pivotal security team responded by adding a whitelisting functionality to Spring AMQP, so that users can now put restrictions on the classes that can be deserialized and mitigate the risk of remote code execution. However, the whitelisting functionality is switched off by default. Consequently, developers who use serialized Java objects as a message format are only protected against such risks if they configure the application correctly. If they have simply upgraded to the latest version of Spring their application is still vulnerable.

New deserialization vulnerability

Using CodeQL, I found another vulnerability that is caused by unsafe deserialization of untrusted user data; the Pivotal team have published a security advisory and patch. The vulnerability has been assigned CVE-2017-8045.

Unlike the previous vulnerability, this weakness affects all Spring AMQP applications, even if the application does not use Java serialized objects at all. Any application that uses Spring AMQP and is accessible by untrusted users should be patched immediately.

Never make the same mistake twice with CodeQL

Although the vulnerability reported by Matthias Kaiser (CVE-2016-2173) was fixed, it is common to see groups of similar issues lurking in a codebase. To find out whether this was the case for Spring AMQP, I wrote a simple query with CodeQL to check whether there was any further unsafe deserialization in the codebase. The goal of the query is to find out if there is any remote input that ends up being deserialized without proper type checking.

Remote input in Spring AMQP is abstracted by the class Message. The Message class contains a private field body that contains the body of the abstracted message. This is the data that is most likely to get deserialized. So the code pattern that we are looking for here is of this form:

byte[] data = msg.getBody();
ObjectInputStream stream = new ObjectInputStream(new ByteArrayInputStream(data));
stream.readObject(); //unsafe

A good place to start is to write some classes that look for the Message class and its methods.

class Message extends RefType {
  Message() {
    this.hasQualifiedName("org.springframework.amqp.core", "Message")
  }
}

/** The method `getBody` */
class GetBodyMethod extends Method {
  GetBodyMethod() {
      this.getDeclaringType() instanceof Message and
      this.hasName("getBody")
  }

Here we defined a QL class that looks for the Message class and its method getBody. If we just run a query to look for the Message class we find where it is declared.

import java

class Message extends RefType {
  Message() {
    this.hasQualifiedName("org.springframework.amqp.core", "Message")
  }
}

from Message msg
select msg

we find where it is declared. We can run a similar query for GetBodyMethod to find where the method is declared. For our purpose, however, we are actually interested in the locations where the field body is being accessed, either directly, or by calling the getBody method. CodeQL provides classes with names like *Access that are abstractions of accesses to fields, variables or methods, etc. So to look for accesses to the field body, we make use of the FieldAccess class:

/** Direct access to the field `body` in `Message` */
class MessageBody extends FieldAccess {
  MessageBody() {
    exists(Field f |
      this.getField() = f and
      f.getDeclaringType() instanceof Message and
      f.hasName("body")
    )
  }
}

This looks for direct accesses to the field body. To look for accesses to the field body via the getBody method, we similarly make use of the MethodAccess class:

/** Access to the method `getBody` in `Message` */
class GetBody extends MethodAccess {
  GetBody() {
      this.getMethod() instanceof GetBodyMethod
  }
}

Running a query to select accesses to the getBody method we find where this method is called in the codebase.

import java

class Message extends RefType {
  Message() {
    this.hasQualifiedName("org.springframework.amqp.core", "Message")
  }
}

/** The method GetBody */
class GetBodyMethod extends Method {
  GetBodyMethod() {
      this.getDeclaringType() instanceof Message and
      this.hasName("getBody")
  }
}

/** Access to the method `getBody` in `Message` */
class GetBody extends MethodAccess {
  GetBody() {
      this.getMethod() instanceof GetBodyMethod
  }
}

from GetBody gb
select gb

As we see, there are rather a lot of places that call this method. We now combine these two sources to form the basis of the tainted input that we want to track:

/** All access to the field `body` in `Message` */
class MessageSource extends FlowSource {
  MessageSource() {
    this instanceof GetBody
    or
    this instanceof MessageBody and
    not exists(Method m | m = this.(FieldAccess).getEnclosingCallable() |
      m instanceof GetBodyMethod
    )
  }
}

Note that we now use the FlowSource class, which has a method call flowsTo that will allow us to track its flow within the code.

With CodeQL, an unsafe deserialization method is abstracted by the UnsafeDeserializationSink in the Deserialization of user-controlled data query. We will use this as the sink to identify any unsafe deserialization. Putting these together, we obtain our final query.

Click to see the final query ```ql import java import semmle.code.java.security.DataFlow class Message extends RefType { Message() { this.hasQualifiedName("org.springframework.amqp.core", "Message") } } /** The method GetBody */ class GetBodyMethod extends Method { GetBodyMethod() { this.getDeclaringType() instanceof Message and this.hasName("getBody") } } /** Access to the method `getBody` in `Message` */ class GetBody extends MethodAccess, FlowSource { GetBody() { this.getMethod() instanceof GetBodyMethod } } /** Direct access to the field `body` in `Message` */ class MessageBody extends FieldAccess, FlowSource { MessageBody() { exists(Field f | this.getField() = f and f.getDeclaringType() instanceof Message and f.hasName("body") ) } } class ObjectInputStreamReadObjectMethod extends Method { ObjectInputStreamReadObjectMethod() { this.getDeclaringType().getASourceSupertype*().hasQualifiedName("java.io", "ObjectInputStream") and (this.hasName("readObject") or this.hasName("readUnshared")) } } predicate unsafeDeserialization(MethodAccess ma, Expr sink) { exists(Method m | m = ma.getMethod() | m instanceof ObjectInputStreamReadObjectMethod and sink = ma.getQualifier() ) } class UnsafeDeserializationSink extends Expr { UnsafeDeserializationSink() { unsafeDeserialization(_, this) } MethodAccess getMethodAccess() { unsafeDeserialization(result, this) } } /** All access to the field `body` in `Message` */ class MessageSource extends FlowSource { MessageSource() { this instanceof GetBody or this instanceof MessageBody and not exists(Method m | m = this.(FieldAccess).getEnclosingCallable() | m instanceof GetBodyMethod ) } } from MessageSource source, UnsafeDeserializationSink sink where source.flowsTo(sink) select source, sink ```

The query finds three results. Two are in test code, but one is very interesting, in which the body of a Message is deserialized in the SerializationUtils class without any type checking. This happens inside the getBodyContentAsString method that is called by the toString method.

While the toString method does not usually get called explicitly, it is common for an object to be implicitly converted to a string by concatenation (using +). Some other methods, like printf also take Java objects as string formatting arguments and convert them into a String by calling toString implicitly. Let’s see if we can find any cases where a toString is called implicitly.

First we look for implicit calls via concatenation to a String. The code pattern to look for here is:

String c = a + msg;

where a is a String and msg is a Message. Concatenation expressions like the example above can be found using the AddExpr class. Since we want one of the Operand expressions (that is, the expression on either side of the +) to be a Message, we put a restriction on the type of the Operand:

from AddExpr add, MessageExpr message
where add.getAnOperand() = message
select message

That’s it! Running the query below, we find 24 locations where this can happen.

Click to see the query ```ql import java import semmle.code.java.StringFormat class Message extends RefType { Message() { this.hasQualifiedName("org.springframework.amqp.core", "Message") } } class MessageExpr extends Expr { MessageExpr() { this.getType() instanceof Message } } from AddExpr add, MessageExpr message where add.getAnOperand() = message select message ```

Now to include the second case, where a Message may be passed as an argument to a call like printf. We use the class FormattingCall, which looks for calls like this in the standard Java libraries:

from AddExpr add, MessageExpr message
where
  add.getAnOperand() = message or
  exists(FormattingCall c | c.getAnArgument() = message)
select message

Running this updated query does not find any more results, but still, we showed that there are 24 places where unsafe deserialization can happen within Spring AMQP itself.

Remote code execution via logging

This discovery was particularly worrying as the toString method in Message is called in many places during error handling and logging, regardless of the intended incoming message format and the converter that is used to convert the message into a Java object. For example, an application may be set up to only accept JSON messages, by using the Jackson2JsonMessageConverter (or a similar custom converter) as a message converter. As the message format and the MessageConverter are both considered safe from deserialization issues, the owner of the application may decide to receive JSON messages directly from the internet. This, however, will result in remote code execution when an attacker sends a message with the Content-Type header set to “application/x-java-serialized-object” and includes a malicious payload generated from Chris Frohoff’s tool as the body. This will cause the message conversion to fail because an unsupported content type is encountered, which throws an exception containing the message, eventually causing toString to be called. By writing a demo application which is a simple modification of Spring’s example application (with the MessageCoverter set to a Jackson2JsonMessageConverter), I was able to trigger an RCE via one of or query results. I then reported this to Pivotal.

Vendor response

Note: Post originally published on LGTM.com on September 20, 2017