You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jf...@apache.org on 2012/10/12 02:43:13 UTC
svn commit: r1397397 - in /thrift/trunk/lib/java:
src/org/apache/thrift/protocol/TCompactProtocol.java
test/org/apache/thrift/protocol/TestTBinaryProtocol.java
test/org/apache/thrift/protocol/TestTCompactProtocol.java
Author: jfarrell
Date: Fri Oct 12 00:43:13 2012
New Revision: 1397397
URL: http://svn.apache.org/viewvc?rev=1397397&view=rev
Log:
THRIFT-1643:Denial of Service attack in TBinaryProtocol.readString
Client: java
Patch: Niraj Tolia
In readString, if the string field's size is greater than the number of bytes remaining in the byte array to deserialize, libthrift will happily allocate a byte array of that size in readStringBody, filling the heap.
Modified:
thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTBinaryProtocol.java
thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTCompactProtocol.java
Modified: thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java?rev=1397397&r1=1397396&r2=1397397&view=diff
==============================================================================
--- thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java (original)
+++ thrift/trunk/lib/java/src/org/apache/thrift/protocol/TCompactProtocol.java Fri Oct 12 00:43:13 2012
@@ -639,15 +639,12 @@ public class TCompactProtocol extends TP
*/
public String readString() throws TException {
int length = readVarint32();
+ checkReadLength(length);
if (length == 0) {
return "";
}
- if (maxNetworkBytes_ != -1 && length > maxNetworkBytes_) {
- throw new TException("Read size greater than max allowed.");
- }
-
try {
if (trans_.getBytesRemainingInBuffer() >= length) {
String str = new String(trans_.getBuffer(), trans_.getBufferPosition(), length, "UTF-8");
@@ -666,12 +663,9 @@ public class TCompactProtocol extends TP
*/
public ByteBuffer readBinary() throws TException {
int length = readVarint32();
+ checkReadLength(length);
if (length == 0) return ByteBuffer.wrap(new byte[0]);
- if (maxNetworkBytes_ != -1 && length > maxNetworkBytes_) {
- throw new TException("Read size greater than max allowed.");
- }
-
byte[] buf = new byte[length];
trans_.readAll(buf, 0, length);
return ByteBuffer.wrap(buf);
@@ -688,6 +682,15 @@ public class TCompactProtocol extends TP
return buf;
}
+ private void checkReadLength(int length) throws TProtocolException {
+ if (length < 0) {
+ throw new TProtocolException("Negative length: " + length);
+ }
+ if (maxNetworkBytes_ != -1 && length > maxNetworkBytes_) {
+ throw new TProtocolException("Length exceeded max allowed: " + length);
+ }
+ }
+
//
// These methods are here for the struct to call, but don't have any wire
// encoding.
Modified: thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTBinaryProtocol.java
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTBinaryProtocol.java?rev=1397397&r1=1397396&r2=1397397&view=diff
==============================================================================
--- thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTBinaryProtocol.java (original)
+++ thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTBinaryProtocol.java Fri Oct 12 00:43:13 2012
@@ -19,6 +19,11 @@
package org.apache.thrift.protocol;
+import org.apache.thrift.TDeserializer;
+import org.apache.thrift.TException;
+
+import thrift.test.Bonk;
+
public class TestTBinaryProtocol extends ProtocolTestBase {
@Override
protected TProtocolFactory getFactory() {
@@ -29,4 +34,17 @@ public class TestTBinaryProtocol extends
protected boolean canBeUsedNaked() {
return true;
}
+
+ public void testOOMDenialOfService() throws Exception {
+ TDeserializer deser = new TDeserializer(new TBinaryProtocol
+ .Factory(false, false, 1000));
+ Bonk bonk = new Bonk();
+ try {
+ // Invalid read length specified here. Would cause an OOM
+ // without the limit on the read length
+ deser.deserialize(bonk, new byte[]{11, 0, 1, 127, -1, -1, -1});
+ } catch (TException e) {
+ // Ignore as we are only checking for OOM in the failure case
+ }
+ }
}
Modified: thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTCompactProtocol.java
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTCompactProtocol.java?rev=1397397&r1=1397396&r2=1397397&view=diff
==============================================================================
--- thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTCompactProtocol.java (original)
+++ thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTCompactProtocol.java Fri Oct 12 00:43:13 2012
@@ -20,6 +20,10 @@
package org.apache.thrift.protocol;
+import org.apache.thrift.TDeserializer;
+import org.apache.thrift.TException;
+
+import thrift.test.Bonk;
public class TestTCompactProtocol extends ProtocolTestBase {
@Override
@@ -32,6 +36,20 @@ public class TestTCompactProtocol extend
return true;
}
+ public void testOOMDenialOfService() throws Exception {
+ // Struct header, Integer.MAX_VALUE length, and only one real
+ // byte of data
+ byte [] bytes = {24, -1, -1, -1, -17, 49};
+ TDeserializer deser = new TDeserializer(new TCompactProtocol
+ .Factory(1000));
+ Bonk bonk = new Bonk();
+ try {
+ deser.deserialize(bonk, bytes);
+ } catch (TException e) {
+ // Ignore as we are only checking for OOM in the failure case
+ }
+ }
+
public static void main(String args[]) throws Exception {
new TestTCompactProtocol().benchmark();
}