Skip to content
Permalink
Browse files Browse the repository at this point in the history
Java: Check the size of the remaining frame before deserializing a st…
…ring

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
  • Loading branch information
stevegury authored and facebook-github-bot committed Dec 6, 2019
1 parent 71c97ff commit 08c2d41
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 14 deletions.
Expand Up @@ -343,6 +343,7 @@ public String readString() throws TException {
}

public String readStringBody(int size) throws TException {
ensureContainerHasEnough(size, TType.BYTE);
checkReadLength(size);
byte[] buf = new byte[size];
trans_.readAll(buf, 0, size);
Expand All @@ -351,6 +352,7 @@ public String readStringBody(int size) throws TException {

public byte[] readBinary() throws TException {
int size = readI32();
ensureContainerHasEnough(size, TType.BYTE);
checkReadLength(size);
byte[] buf = new byte[size];
trans_.readAll(buf, 0, size);
Expand Down
Expand Up @@ -638,6 +638,7 @@ private byte[] readBinary(int length) throws TException {
return new byte[0];
}

ensureContainerHasEnough(length, TType.BYTE);
byte[] buf = new byte[length];
trans_.readAll(buf, 0, length);
return buf;
Expand Down
4 changes: 4 additions & 0 deletions thrift/lib/java/src/resources/testing_data_structures.thrift
Expand Up @@ -62,3 +62,7 @@ struct MySetStruct {
struct MyMapStruct {
1: map<i64, string> mapping
}

struct MyStringStruct {
1: string aLongString
}
Expand Up @@ -19,6 +19,7 @@
import com.facebook.thrift.java.test.MyListStruct;
import com.facebook.thrift.java.test.MyMapStruct;
import com.facebook.thrift.java.test.MySetStruct;
import com.facebook.thrift.java.test.MyStringStruct;
import com.facebook.thrift.protocol.TBinaryProtocol;
import com.facebook.thrift.protocol.TCompactProtocol;
import com.facebook.thrift.protocol.TProtocol;
Expand Down Expand Up @@ -52,15 +53,15 @@ public class TruncatedFrameTest extends junit.framework.TestCase {
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x01, // value = 2L
(byte) 0x02, // value = 2L
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x01, // value = 3L
(byte) 0x03, // value = 3L
(byte) 0x00, // Stop
};

Expand Down Expand Up @@ -139,15 +140,15 @@ public static void testLongListCompact() throws Exception {
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x01, // value = 2L
(byte) 0x02, // value = 2L
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x00,
(byte) 0x01, // value = 3L
(byte) 0x03, // value = 3L
(byte) 0x00, // Stop
};

Expand Down Expand Up @@ -273,16 +274,61 @@ public static void testMapCompact() throws Exception {
testTruncated(new MyMapStruct(), iprot);
}

private static final char[] hexArray = "0123456789ABCDEF".toCharArray();
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 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;
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);
}
}

0 comments on commit 08c2d41

Please sign in to comment.