CVE-2019-11938 (Java, published 2020-03-10T21:15Z)
Description: value: Java Facebook Thrift servers would not error upon receiving messages declaring containers of sizes larger than the payload. As a result, malicious clients could send short messages which would result in a large memory allocation, potentially leading to denial of service. This issue affects Facebook Thrift prior to v2019.12.09.00.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-770 \u2014 Allocation of Resources Without Limits or Throttling
The product allocates a reusable resource or group of resources on behalf of an actor without imposing any restrictions on the size or number of resources that can be allocated, in violation of the intended security policy for that actor.
Repository: https://github.com/facebook/fbthrift
Files changed:
thrift/lib/java/src/main/java/com/facebook/thrift/protocol/TBinaryProtocol.java
thrift/lib/java/src/main/java/com/facebook/thrift/protocol/TCompactProtocol.java
thrift/lib/java/src/resources/testing_data_structures.thrift
thrift/lib/java/src/test/java/com/facebook/thrift/TruncatedFrameTest.java
Commit message: Java: Check the size of the remaining frame before deserializing a string

Summary:
In order to avoid over-allocating memory for malformed or truncated frame, we
ensure that we have enough data in the current frame.

This is a partial fix for CVE-2019-11938.

Reviewed By: vitaut

Differential Revision: D14505601

fbshipit-source-id: c90f248828b067a3a5debcc8df6a3f4e9da6d195


Vulnerable code:
    (byte) 0x01, // value = 2L
    (byte) 0x01, // value = 3L
    (byte) 0x01, // value = 2L
    (byte) 0x01, // value = 3L
  private static final char[] hexArray = "0123456789ABCDEF".toCharArray();
  private static String bytesToHex(byte[] bytes, int length) {
    String out = "";
    for (int j = 0; j < length; j++) {
      int v = bytes[j] & 0xFF;
      out += hexArray[v >>> 4];
      out += hexArray[v & 0x0F];
      out += " ";
    }
    return out;

Fixed code:
    ensureContainerHasEnough(size, TType.BYTE);
    ensureContainerHasEnough(size, TType.BYTE);
    ensureContainerHasEnough(length, TType.BYTE);

struct MyStringStruct {
  1: string aLongString
}
import com.facebook.thrift.java.test.MyStringStruct;
    (byte) 0x02, // value = 2L
    (byte) 0x03, // value = 3L
    (byte) 0x02, // value = 2L
    (byte) 0x03, // value = 3L
  private static final byte[] kBinaryStringEncoding = {
    TType.STRING, // Field Type = string
    (byte) 0x00,
    (byte) 0x01, // Field id = 1
    (byte) 0x00,
    (byte) 0x00,
    (byte) 0x00,
    (byte) 0xFF, // string length (255!)
    (byte) 0x48,
    (byte) 0x65,
    (byte) 0x6C,
    (byte) 0x6C,
    (byte) 0x6F,
    (byte) 0x2C,
    (byte) 0x20,
    (byte) 0x57,
    (byte) 0x6F,
    (byte) 0x72,
    (byte) 0x6C,
    (byte) 0x64,
    (byte) 0x21, // string chars: "Hello, World!"
    (byte) 0x00, // Stop
  };
  private static final byte[] kCompactStringEncoding = {
    (byte) 0b00011000, // field id delta (0001) + type (1000) = Binary
    (byte) 0xFF,
    (byte) 0x0F, // string size (varint) = 0x0FFF (4095)
    (byte) 0x48,
    (byte) 0x65,
    (byte) 0x6C,
    (byte) 0x6C,
    (byte) 0x6F,
    (byte) 0x2C,
    (byte) 0x20,
    (byte) 0x57,
    (byte) 0x6F,
    (byte) 0x72,
    (byte) 0x6C,
    (byte) 0x64,
    (byte) 0x21, // string chars: "Hello, World!"
    (byte) 0x00, // Stop
  };

  @Test
  public void testStringBinary() throws Exception {
    TMemoryInputTransport buf = new TMemoryInputTransport(kBinaryStringEncoding);
    TProtocol iprot = new TBinaryProtocol(buf);
    testTruncated(new MyStringStruct(), iprot);
  }

  @Test
  public void testStringCompact() throws Exception {
    TMemoryInputTransport buf = new TMemoryInputTransport(kCompactStringEncoding);
    TProtocol iprot = new TCompactProtocol(buf);
    testTruncated(new MyStringStruct(), iprot);