Skip to content

Commit

Permalink
further work on in memory loading checks for ASN.1 objects
Browse files Browse the repository at this point in the history
  • Loading branch information
dghgit committed Sep 29, 2019
1 parent f8fdfaf commit b1bc752
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ protected ASN1Primitive buildObject(
{
boolean isConstructed = (tag & CONSTRUCTED) != 0;

DefiniteLengthInputStream defIn = new DefiniteLengthInputStream(this, length);
DefiniteLengthInputStream defIn = new DefiniteLengthInputStream(this, length, limit);

if ((tag & APPLICATION) != 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,7 @@ public ASN1Encodable readObject()
}
else
{
if (length >= _limit) // after all we must have read at least 1 byte
{
throw new IOException("corrupted stream - out of bounds length found: " + length + " >= " + _limit);
}
DefiniteLengthInputStream defIn = new DefiniteLengthInputStream(_in, length);
DefiniteLengthInputStream defIn = new DefiniteLengthInputStream(_in, length, _limit);

if ((tag & BERTags.APPLICATION) != 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ class DefiniteLengthInputStream
private static final byte[] EMPTY_BYTES = new byte[0];

private final int _originalLength;

private int _remaining;

DefiniteLengthInputStream(
InputStream in,
int length)
int length,
int limit)
{
super(in, length);
super(in, limit, length);

if (length < 0)
{
Expand Down Expand Up @@ -97,6 +99,12 @@ byte[] toByteArray()
return EMPTY_BYTES;
}

// make sure it's safe to do this!
if (_remaining >= this.getLimit())
{
throw new IOException("corrupted stream - out of bounds length found: " + _remaining + " >= " + this.getLimit());
}

byte[] bytes = new byte[_remaining];
if ((_remaining -= Streams.readFully(_in, bytes)) != 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class IndefiniteLengthInputStream
int limit)
throws IOException
{
super(in, limit);
super(in, limit, limit);

_b1 = in.read();
_b2 = in.read();
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/org/bouncycastle/asn1/LimitedInputStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,27 @@ abstract class LimitedInputStream
{
protected final InputStream _in;
private int _limit;
private int _length;

LimitedInputStream(
InputStream in,
int limit)
int limit,
int length)
{
this._in = in;
this._limit = limit;
this._length = length;
}

int getLimit()
{
return _limit;
}

int getRemaining()
{
// TODO: maybe one day this can become more accurate
return _limit;
return _length;
}

protected void setParentEofDetect(boolean on)
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/bouncycastle/asn1/StreamUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class StreamUtil
private static final long MAX_MEMORY = Runtime.getRuntime().maxMemory();

/**
* Find out possible longest length...
* Find out possible longest length, capped by available memory.
*
* @param in input stream of interest
* @return length calculation or MAX_VALUE.
Expand All @@ -20,7 +20,7 @@ static int findLimit(InputStream in)
{
if (in instanceof LimitedInputStream)
{
return ((LimitedInputStream)in).getRemaining();
return ((LimitedInputStream)in).getLimit();
}
else if (in instanceof ASN1InputStream)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ public class InputStreamTest

private static final byte[] classCast1 = Base64.decode("p1AkHmYAvfOEIrL4ESfrNg==");

private static final byte[] memoryError1 = Base64.decode("JICNbaBUTTq7uxj5mg==");
private static final byte[] memoryError2 = Base64.decode("vm66gOiEe+FV/NvujMwSkUp5Lffw5caQlaRU5sdMPC70IGWmyK2/");
private static final byte[] memoryError3 = Base64.decode("vm4ogOSEfVGsS3w+KTzb2A0ALYR8VBOQqQeuRwnsPC4AAGWEDLjd");
private static final byte[] classCast2 = Base64.decode("JICNbaBUTTq7uxj5mg==");
private static final byte[] memoryError1 = Base64.decode("vm66gOiEe+FV/NvujMwSkUp5Lffw5caQlaRU5sdMPC70IGWmyK2/");
private static final byte[] memoryError2 = Base64.decode("vm4ogOSEfVGsS3w+KTzb2A0ALYR8VBOQqQeuRwnsPC4AAGWEDLjd");

public String getName()
{
Expand Down Expand Up @@ -75,10 +75,10 @@ public void performTest()
}

testWithByteArray(classCast1, "unknown object encountered: class org.bouncycastle.asn1.DLApplicationSpecific");

testWithByteArray(memoryError1, "corrupted stream - out of bounds length found: 109 >= 13");
testWithByteArray(memoryError2, "corrupted stream - out of bounds length found: 2078365180 >= 110");
testWithByteArray(memoryError3, "corrupted stream - out of bounds length found: 2102504523 >= 110");
testWithByteArray(classCast2, "unknown object encountered: class org.bouncycastle.asn1.BERTaggedObjectParser");

testWithByteArray(memoryError1, "corrupted stream - out of bounds length found: 2078365180 >= 110");
testWithByteArray(memoryError2, "corrupted stream - out of bounds length found: 2102504523 >= 110");
}

private void testWithByteArray(byte[] data, String message)
Expand Down
10 changes: 5 additions & 5 deletions pkix/src/test/java/org/bouncycastle/openssl/test/ParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,15 @@ public void performTest()
doOpenSslDsaTest("rc2_64_cbc");
doOpenSslRsaTest("rc2_64_cbc");

doDudPasswordTest("7fd98", 0, "corrupted stream - out of bounds length found: 599005160 >= 19");
doDudPasswordTest("ef677", 1, "corrupted stream - out of bounds length found: 2087569732 >= 66");
doDudPasswordTest("7fd98", 0, "corrupted stream - out of bounds length found: 599005160 >= 447");
doDudPasswordTest("ef677", 1, "corrupted stream - out of bounds length found: 2087569732 >= 447");
doDudPasswordTest("800ce", 2, "unknown tag 26 encountered");
doDudPasswordTest("b6cd8", 3, "DEF length 81 object truncated by 56");
doDudPasswordTest("28ce09", 4, "DEF length 110 object truncated by 28");
doDudPasswordTest("2ac3b9", 5, "DER length more than 4 bytes: 11");
doDudPasswordTest("2cba96", 6, "corrupted stream - out of bounds length found: 100 >= 67");
doDudPasswordTest("2e3354", 7, "corrupted stream - out of bounds length found: 42 >= 35");
doDudPasswordTest("2f4142", 8, "corrupted stream - out of bounds length found: 127 >= 39");
doDudPasswordTest("2cba96", 6, "DEF length 100 object truncated by 35");
doDudPasswordTest("2e3354", 7, "DEF length 42 object truncated by 9");
doDudPasswordTest("2f4142", 8, "DER length more than 4 bytes: 14");
doDudPasswordTest("2fe9bb", 9, "DER length more than 4 bytes: 65");
doDudPasswordTest("3ee7a8", 10, "DER length more than 4 bytes: 57");
doDudPasswordTest("41af75", 11, "unknown tag 16 encountered");
Expand Down

0 comments on commit b1bc752

Please sign in to comment.