Skip to content

Commit a6c3de2

Browse files
committed
github #389 - fixed escaping ArrayIndexOutOfBoundException on parsing dodgy armored data, added better checking for valid Base64 in armoring.
1 parent 1fd3f69 commit a6c3de2

File tree

3 files changed

+96
-5
lines changed

3 files changed

+96
-5
lines changed

pg/src/main/java/org/bouncycastle/bcpg/ArmoredInputStream.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
import java.io.EOFException;
44
import java.io.IOException;
55
import java.io.InputStream;
6-
import java.util.Arrays;
7-
import java.util.Vector;
86

97
import org.bouncycastle.util.StringList;
108
import org.bouncycastle.util.Strings;
@@ -26,6 +24,11 @@ public class ArmoredInputStream
2624
{
2725
decodingTable = new byte[128];
2826

27+
for (int i = 0; i < decodingTable.length; i++)
28+
{
29+
decodingTable[i] = (byte)0xff;
30+
}
31+
2932
for (int i = 'A'; i <= 'Z'; i++)
3033
{
3134
decodingTable[i] = (byte)(i - 'A');
@@ -56,7 +59,7 @@ private int decode(
5659
int in2,
5760
int in3,
5861
int[] out)
59-
throws EOFException
62+
throws IOException
6063
{
6164
int b1, b2, b3, b4;
6265

@@ -70,6 +73,11 @@ private int decode(
7073
b1 = decodingTable[in0] &0xff;
7174
b2 = decodingTable[in1] & 0xff;
7275

76+
if ((b1 | b2) < 0)
77+
{
78+
throw new IOException("invalid armor");
79+
}
80+
7381
out[2] = ((b1 << 2) | (b2 >> 4)) & 0xff;
7482

7583
return 2;
@@ -80,6 +88,11 @@ else if (in3 == '=')
8088
b2 = decodingTable[in1];
8189
b3 = decodingTable[in2];
8290

91+
if ((b1 | b2 | b3) < 0)
92+
{
93+
throw new IOException("invalid armor");
94+
}
95+
8396
out[1] = ((b1 << 2) | (b2 >> 4)) & 0xff;
8497
out[2] = ((b2 << 4) | (b3 >> 2)) & 0xff;
8598

@@ -92,6 +105,11 @@ else if (in3 == '=')
92105
b3 = decodingTable[in2];
93106
b4 = decodingTable[in3];
94107

108+
if ((b1 | b2 | b3 | b4) < 0)
109+
{
110+
throw new IOException("invalid armor");
111+
}
112+
95113
out[0] = ((b1 << 2) | (b2 >> 4)) & 0xff;
96114
out[1] = ((b2 << 4) | (b3 >> 2)) & 0xff;
97115
out[2] = ((b3 << 6) | b4) & 0xff;
@@ -308,7 +326,12 @@ private int readIgnoreSpace()
308326
{
309327
c = in.read();
310328
}
311-
329+
330+
if (c >= 128)
331+
{
332+
throw new IOException("invalid armor");
333+
}
334+
312335
return c;
313336
}
314337

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package org.bouncycastle.openpgp.test;
2+
3+
import java.io.ByteArrayInputStream;
4+
import java.io.IOException;
5+
import java.security.Security;
6+
7+
import org.bouncycastle.bcpg.ArmoredInputStream;
8+
import org.bouncycastle.jce.provider.BouncyCastleProvider;
9+
import org.bouncycastle.openpgp.PGPObjectFactory;
10+
import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
11+
import org.bouncycastle.util.encoders.Hex;
12+
import org.bouncycastle.util.test.SimpleTest;
13+
14+
public class ArmoredInputStreamTest
15+
extends SimpleTest
16+
{
17+
private static final byte[] bogusData = Hex.decode(
18+
"ed864b3c622d5d71d43e5bd77876e81bfaaba8522f64cb494dc897daa3494f7da598f5907b758b72394fbefea77b86a16865e7bf" +
19+
"b8f5bb46bb0d2db4a99a6a4542b9040a0e4f74b8e202c4eb255e8a81a59be9c0d5d2c593b8b512c9bdc75a243cb0992b5a885889" +
20+
"a4a3d3d70e1fcb415d4f718e8230b11895e3706314912554d7c19dafb733df7d02e9a2f42492139648618b1943af9e2941bd0e42" +
21+
"73b58de9b734d15a793a6d7673b3e90dedebc2a479965680de61880dddea25c0168237a6e52846e6a5aa9fb9161ac7a3996315cf" +
22+
"7d391cb86e86cce44e2a353b68cf84d3ac49eddde9a040180533af4aade92de7a03c1982020a2591141aeffd2ad07ffbc0d0a303" +
23+
"710763f47317a5c468d18e69e4094945060f707778cd65b4e94c2e27b5e5a8f4d510e01c9f84b22e1c59486a9b72552682833a07" +
24+
"23995febde56a59d31b60b2cdac00efbf693ac27607c8c4a502749bc7bde65ec80c661aadada4611e3093607d8c7927bf4b29ea9" +
25+
"0481a4616952abb88cb2f4ad78c2e94ba9193f0d0dac17d972ed0b6a738a11a6f27a3a5857e9a0746c5acb2a33ea05491e23db39" +
26+
"657549b5b1a341f2088232b5c72c1a3fcce6dd8b1ce8ade51977521e473b6b208458ce513606daf47689c9a239e161cc592f070f" +
27+
"395a2b964547954516651fc122a781b336d3ddc529c37c16022e6882ba52a6ff9ffd1e362971096997053e916953ca6146eb9973" +
28+
"a28663074692fa3d216b4f169d20e32655602461dce525db1d94b9c43620e05a9e2d3465bb7ed0c07493738434bcf058a73b22ab" +
29+
"0fd6ade1b995b3129d791e3f3a9446d35d9fe521ba5f196f98e7b637132aaa2df424ddb4e372cb70ede1eacdf0b454de91ae279f" +
30+
"ec3c16e268f262a169a2d9ecb27ac1ae177fa6f31f63991179b35e5a48d5cd0e369f50f92cf0327bde1cbd8125201b0c0e4c9242" +
31+
"d416cf48a9ac9c367ab424999fdf3cf5faac259b302b68c417f1461380a57d4f6a5bad8e60ac2140af53f3b44b2e4a44383e597e" +
32+
"f0d7594d2f8c74346a7759b13364bf7abe08663ed4d2ab92bd60231d71e63c125739c446c048e76b7157c644ad6e136f3078c79a" +
33+
"27d505af22b25706c4cf1aec5cb9578e0d0f0471fabdbfc4504a4f971b2c68a4af75ff9a438b1e4c3507dd93c38dc8caa43e87c3" +
34+
"95e27d9b23402671e1a8a6f9563ba8e9d00d2f99d77d25bdda2ac4fe363db138a6eeea4ec1a5ae104cc30b9e4f468799335157da" +
35+
"a9f4c310ef0806ef1803d81db84f58b45639a1749c705594cf5dbbe6109af4711eb080af4edd0d0386c09676b705d3a0ccad5cc7" +
36+
"b5f289a884ce649b5b00b46ad33ee43b0db8c0202cf1fdde4c3b61d5fec99e3024016ccdb0ff2d321f08781d08e4312de38245eb" +
37+
"bc2af032d2a59e36be6467bc23456b4ac178d36cf9f45df5e833a1981ed1a1032679ea0a");
38+
39+
public String getName()
40+
{
41+
return "ArmoredInputStream";
42+
}
43+
44+
public void performTest()
45+
throws Exception
46+
{
47+
try
48+
{
49+
PGPObjectFactory pgpObjectFactoryOfTestFile = new PGPObjectFactory(
50+
new ArmoredInputStream(new ByteArrayInputStream(bogusData)), new JcaKeyFingerprintCalculator());
51+
pgpObjectFactoryOfTestFile.nextObject(); // <-- EXCEPTION HERE
52+
fail("no exception");
53+
}
54+
catch (IOException e)
55+
{
56+
isTrue("invalid armor".equals(e.getMessage()));
57+
}
58+
}
59+
60+
public static void main(
61+
String[] args)
62+
{
63+
Security.addProvider(new BouncyCastleProvider());
64+
65+
runTest(new ArmoredInputStreamTest());
66+
}
67+
}

pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ public class RegressionTest
2929
new PGPECDHTest(),
3030
new PGPECMessageTest(),
3131
new PGPParsingTest(),
32-
new SExprTest()
32+
new SExprTest(),
33+
new ArmoredInputStreamTest()
3334
};
3435

3536
public static void main(

0 commit comments

Comments
 (0)