Skip to content

Commit

Permalink
Fix potential large memory allocation in DerInputStream (#43)
Browse files Browse the repository at this point in the history
* Fix potential large memory allocation in DerInputStream
The object length limit check was adding 2 int values which could result in an overflow exception, allowing very large lengths to bypass the limit enforcement.
Now the limit check casts the value to a long before doing addition to avoid integer overflow.
  • Loading branch information
nhartner authored Jul 9, 2024
1 parent f50ad63 commit 2efc064
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public DerObject readObject(int limit, AtomicInteger bytesRead)
DerObject obj = new DerObject();
obj.setTag(readTag(innerBytesRead));
obj.setLength(readLength(innerBytesRead));
if (innerBytesRead.get() + obj.getLength() > limit) {
if ((long) innerBytesRead.get() + obj.getLength() > limit) {
throw new DerEncodingException(
"Object length [" + obj.getLength() + "] is larger than allowed.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@
* =========================LICENSE_END==================================
*/

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.fail;

import com.google.common.collect.Lists;
import com.ripple.cryptoconditions.der.DerEncodingException;
import com.ripple.cryptoconditions.helpers.TestKeyFactory;
import net.i2p.crypto.eddsa.EdDSAEngine;
import net.i2p.crypto.eddsa.EdDSAPublicKey;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.util.encoders.Hex;
import org.junit.BeforeClass;
import org.junit.Test;

Expand Down Expand Up @@ -138,6 +142,33 @@ public void readWriteEd25519Condition() throws Exception {
assertThat(readAndWrittenCondition, is(ed25519Condition));
}

@Test
public void testDeserializeLengthOverflow() throws Exception {
String conditionHexDer = "A406800161810100";
byte [] conditionBytes = Hex.decode(conditionHexDer);
Condition readCondition =
CryptoConditionReader.readCondition(conditionBytes);
assertThat(readCondition.getFingerprintBase64Url(), is("YQ"));

// Replace object length (fourth byte 01) with 84 7FFFFFFF
// 84 is DER encoding for length of length
// 7FFFFFFF is Integer.MAX_INT
// Unchecked, this would cause OutOfMemoryError: Requested array size exceeds VM limit
String overflowConditionHexDer =
conditionHexDer.substring(0, 6) + "847FFFFFFF" + conditionHexDer.substring(8);

byte [] overflowConditionBytes = Hex.decode(overflowConditionHexDer);

try {
CryptoConditionReader.readCondition(overflowConditionBytes);
fail("DerEncodingException expected");
} catch (DerEncodingException encodingException) {
// expect
assertThat(encodingException.getMessage(),
containsString(Integer.MAX_VALUE + "] is larger than allowed"));
}
}

@Test
public void readWriteThresholdCondition() throws Exception {
final Condition readAndWrittenCondition = CryptoConditionReader
Expand Down

0 comments on commit 2efc064

Please # to comment.