Fix buffer underflows in cipher light weight API input/output streams and beef up testing.

Buffer underflows could occur when:
- decrypting data > internal buffer size in output stream (input stream was fixed in prior commit)
- packet mode AE cipher (e.g. CCM) is used with a data size > internal buffer size (since all output is buffered)

Buffer is now sized appropriately to every cipher operation immediately prior to it (using getUpdateOutputSize/getOutputSize as appropriate) in both streams.

Tests now run over boundaries of various block/buffer sizes to try to expose issues (0, 64 bit block, 128 bit block, 1K, 2K, 4K).

5 files changed, 127 insertions, 59 deletions

diff --git a/core/src/main/java/org/bouncycastle/crypto/io/CipherIOException.java b/core/src/main/java/org/bouncycastle/crypto/io/CipherIOException.java
new file mode 100644
index 00000000..beeb60bc
--- /dev/null
+++ b/core/src/main/java/org/bouncycastle/crypto/io/CipherIOException.java
@@ -0,0 +1,26 @@
+package org.bouncycastle.crypto.io;
+
+import java.io.IOException;
+
+/**
+ * {@link IOException} wrapper around an exception indicating a problem with the use of a cipher.
+ */
+public class CipherIOException
+    extends IOException
+{
+    private static final long serialVersionUID = 1L;
+
+    private final Throwable cause;
+
+    public CipherIOException(String message, Throwable cause)
+    {
+        super(message);
+
+        this.cause = cause;
+    }
+
+    public Throwable getCause()
+    {
+        return cause;
+    }
+}
\ No newline at end of file
diff --git a/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java b/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java
index 66772df5..f80add1a 100644
--- a/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java
+++ b/core/src/main/java/org/bouncycastle/crypto/io/CipherInputStream.java
@@ -12,28 +12,28 @@ import org.bouncycastle.crypto.modes.AEADBlockCipher;
 /**
  * A CipherInputStream is composed of an InputStream and a cipher so that read() methods return data
  * that are read in from the underlying InputStream but have been additionally processed by the
- * Cipher. The cipher must be fully initialized before being used by a CipherInputStream.
+ * cipher. The cipher must be fully initialized before being used by a CipherInputStream.
  * <p/>
- * For example, if the Cipher is initialized for decryption, the
+ * For example, if the cipher is initialized for decryption, the
  * CipherInputStream will attempt to read in data and decrypt them,
  * before returning the decrypted data.
  */
 public class CipherInputStream
     extends FilterInputStream
 {
+    private static final int INPUT_BUF_SIZE = 2048;
+
     private BufferedBlockCipher bufferedBlockCipher;
     private StreamCipher streamCipher;
     private AEADBlockCipher aeadBlockCipher;
 
-    private final byte[] buf;
-    private final byte[] inBuf;
+    private byte[] buf;
+    private final byte[] inBuf = new byte[INPUT_BUF_SIZE];
 
     private int bufOff;
     private int maxBuf;
     private boolean finalized;
 
-    private static final int INPUT_BUF_SIZE = 2048;
-
     /**
      * Constructs a CipherInputStream from an InputStream and a
      * BufferedBlockCipher.
@@ -45,11 +45,6 @@ public class CipherInputStream
         super(is);
 
         this.bufferedBlockCipher = cipher;
-
-        int outSize = cipher.getOutputSize(INPUT_BUF_SIZE);
-
-        buf = new byte[(outSize > INPUT_BUF_SIZE) ? outSize : INPUT_BUF_SIZE];
-        inBuf = new byte[INPUT_BUF_SIZE];
     }
 
     public CipherInputStream(
@@ -59,9 +54,6 @@ public class CipherInputStream
         super(is);
 
         this.streamCipher = cipher;
-
-        buf = new byte[INPUT_BUF_SIZE];
-        inBuf = new byte[INPUT_BUF_SIZE];
     }
 
     /**
@@ -72,11 +64,6 @@ public class CipherInputStream
         super(is);
 
         this.aeadBlockCipher = cipher;
-
-        int outSize = cipher.getOutputSize(INPUT_BUF_SIZE);
-
-        buf = new byte[(outSize > INPUT_BUF_SIZE) ? outSize : INPUT_BUF_SIZE];
-        inBuf = new byte[INPUT_BUF_SIZE];
     }
 
     /**
@@ -112,6 +99,7 @@ public class CipherInputStream
 
             try
             {
+                ensureCapacity(read, false);
                 if (bufferedBlockCipher != null)
                 {
                     maxBuf = bufferedBlockCipher.processBytes(inBuf, 0, read, buf, 0);
@@ -128,7 +116,7 @@ public class CipherInputStream
             }
             catch (Exception e)
             {
-                throw new IOException("Error processing stream " + e);
+                throw new CipherIOException("Error processing stream ", e);
             }
         }
         return maxBuf;
@@ -140,6 +128,7 @@ public class CipherInputStream
         try
         {
             finalized = true;
+            ensureCapacity(0, true);
             if (bufferedBlockCipher != null)
             {
                 maxBuf = bufferedBlockCipher.doFinal(buf, 0);
@@ -159,7 +148,7 @@ public class CipherInputStream
         }
         catch (Exception e)
         {
-            throw new IOException("Error finalising cipher " + e);
+            throw new CipherIOException("Error finalising cipher ", e);
         }
     }
 
@@ -263,11 +252,49 @@ public class CipherInputStream
     }
 
     /**
+     * Ensure the ciphertext buffer has space sufficient to accept an upcoming output.
+     *
+     * @param updateSize the size of the pending update.
+     * @param <code>true</code> iff this the cipher is to be finalised.
+     */
+    private void ensureCapacity(int updateSize, boolean finalOutput)
+    {
+        int bufLen = updateSize;
+        if (finalOutput)
+        {
+            if (bufferedBlockCipher != null)
+            {
+                bufLen = bufferedBlockCipher.getOutputSize(updateSize);
+            }
+            else if (aeadBlockCipher != null)
+            {
+                bufLen = aeadBlockCipher.getOutputSize(updateSize);
+            }
+        }
+        else
+        {
+            if (bufferedBlockCipher != null)
+            {
+                bufLen = bufferedBlockCipher.getUpdateOutputSize(updateSize);
+            }
+            else if (aeadBlockCipher != null)
+            {
+                bufLen = aeadBlockCipher.getUpdateOutputSize(updateSize);
+            }
+        }
+
+        if ((buf == null) || (buf.length < bufLen))
+        {
+            buf = new byte[bufLen];
+        }
+    }
+
+    /**
      * Closes the underlying input stream and finalises the processing of the data by the cipher.
      *
      * @throws IOException if there was an error closing the input stream.
      * @throws InvalidCipherTextIOException if the data read from the stream was invalid ciphertext
-     * (e.g. the cipher is an AEAD cipher and the ciphertext tag check fails).
+     *             (e.g. the cipher is an AEAD cipher and the ciphertext tag check fails).
      */
     public void close()
         throws IOException
diff --git a/core/src/main/java/org/bouncycastle/crypto/io/CipherOutputStream.java b/core/src/main/java/org/bouncycastle/crypto/io/CipherOutputStream.java
index 9beb5b95..2fe95bed 100644
--- a/core/src/main/java/org/bouncycastle/crypto/io/CipherOutputStream.java
+++ b/core/src/main/java/org/bouncycastle/crypto/io/CipherOutputStream.java
@@ -118,7 +118,7 @@ public class CipherOutputStream
         int len)
         throws IOException
     {
-        ensureCapacity(len);
+        ensureCapacity(len, false);
 
         if (bufferedBlockCipher != null)
         {
@@ -149,24 +149,35 @@ public class CipherOutputStream
     /**
      * Ensure the ciphertext buffer has space sufficient to accept an upcoming output.
      *
-     * @param outputSize the size of the pending update.
+     * @param updateSize the size of the pending update.
+     * @param <code>true</code> iff this the cipher is to be finalised.
      */
-    private void ensureCapacity(int outputSize)
+    private void ensureCapacity(int updateSize, boolean finalOutput)
     {
-        // This overestimates buffer on updates for AEAD/padded, but keeps it simple.
-        int bufLen;
-        if (bufferedBlockCipher != null)
+        int bufLen = updateSize;
+        if (finalOutput)
         {
-            bufLen = bufferedBlockCipher.getOutputSize(outputSize);
-        }
-        else if (aeadBlockCipher != null)
-        {
-            bufLen = aeadBlockCipher.getOutputSize(outputSize);
+            if (bufferedBlockCipher != null)
+            {
+                bufLen = bufferedBlockCipher.getOutputSize(updateSize);
+            }
+            else if (aeadBlockCipher != null)
+            {
+                bufLen = aeadBlockCipher.getOutputSize(updateSize);
+            }
         }
         else
         {
-            bufLen = outputSize;
+            if (bufferedBlockCipher != null)
+            {
+                bufLen = bufferedBlockCipher.getUpdateOutputSize(updateSize);
+            }
+            else if (aeadBlockCipher != null)
+            {
+                bufLen = aeadBlockCipher.getUpdateOutputSize(updateSize);
+            }
         }
+
         if ((buf == null) || (buf.length < bufLen))
         {
             buf = new byte[bufLen];
@@ -213,7 +224,7 @@ public class CipherOutputStream
     public void close()
         throws IOException
     {
-        ensureCapacity(0);
+        ensureCapacity(0, true);
         IOException error = null;
         try
         {
@@ -242,7 +253,7 @@ public class CipherOutputStream
         }
         catch (Exception e)
         {
-            error = new IOException("Error closing stream: " + e);
+            error = new CipherIOException("Error closing stream: ", e);
         }
 
         try
diff --git a/core/src/main/java/org/bouncycastle/crypto/io/InvalidCipherTextIOException.java b/core/src/main/java/org/bouncycastle/crypto/io/InvalidCipherTextIOException.java
index b601d4c1..46561c67 100644
--- a/core/src/main/java/org/bouncycastle/crypto/io/InvalidCipherTextIOException.java
+++ b/core/src/main/java/org/bouncycastle/crypto/io/InvalidCipherTextIOException.java
@@ -8,21 +8,12 @@ import java.io.IOException;
  * expose invalid ciphertext errors.
  */
 public class InvalidCipherTextIOException
-    extends IOException
+    extends CipherIOException
 {
     private static final long serialVersionUID = 1L;
 
-    private final Throwable cause;
-
     public InvalidCipherTextIOException(String message, Throwable cause)
     {
-        super(message);
-
-        this.cause = cause;
-    }
-
-    public Throwable getCause()
-    {
-        return cause;
+        super(message, cause);
     }
 }
\ No newline at end of file
diff --git a/core/src/test/java/org/bouncycastle/crypto/test/CipherStreamTest.java b/core/src/test/java/org/bouncycastle/crypto/test/CipherStreamTest.java
index 5c85a73f..ca384962 100644
--- a/core/src/test/java/org/bouncycastle/crypto/test/CipherStreamTest.java
+++ b/core/src/test/java/org/bouncycastle/crypto/test/CipherStreamTest.java
@@ -56,6 +56,8 @@ import org.bouncycastle.util.test.SimpleTest;
 public class CipherStreamTest
     extends SimpleTest
 {
+    private int streamSize;
+
     public String getName()
     {
         return "CipherStreamTest";
@@ -124,8 +126,8 @@ public class CipherStreamTest
     {
         cipher.init(true, params);
 
-        byte[] ciphertext = new byte[cipher.getOutputSize(1000)];
-        cipher.doFinal(ciphertext, cipher.processBytes(new byte[1000], 0, 1000, ciphertext, 0));
+        byte[] ciphertext = new byte[cipher.getOutputSize(streamSize)];
+        cipher.doFinal(ciphertext, cipher.processBytes(new byte[streamSize], 0, streamSize, ciphertext, 0));
 
         // Tamper
         ciphertext[0] += 1;
@@ -162,11 +164,11 @@ public class CipherStreamTest
     {
         cipher.init(true, params);
 
-        byte[] ciphertext = new byte[cipher.getOutputSize(1000)];
-        cipher.doFinal(ciphertext, cipher.processBytes(new byte[1000], 0, 1000, ciphertext, 0));
+        byte[] ciphertext = new byte[cipher.getOutputSize(streamSize)];
+        cipher.doFinal(ciphertext, cipher.processBytes(new byte[streamSize], 0, streamSize, ciphertext, 0));
 
         // Truncate to just smaller than complete tag
-        byte[] truncated = new byte[ciphertext.length - 1000 - 1];
+        byte[] truncated = new byte[ciphertext.length - streamSize - 1];
         System.arraycopy(ciphertext, 0, truncated, 0, truncated.length);
 
         cipher.init(false, params);
@@ -212,8 +214,8 @@ public class CipherStreamTest
     {
         cipher.init(true, params);
 
-        byte[] ciphertext = new byte[cipher.getOutputSize(1000)];
-        cipher.doFinal(ciphertext, cipher.processBytes(new byte[1000], 0, 1000, ciphertext, 0));
+        byte[] ciphertext = new byte[cipher.getOutputSize(streamSize)];
+        cipher.doFinal(ciphertext, cipher.processBytes(new byte[streamSize], 0, streamSize, ciphertext, 0));
 
         // Tamper
         ciphertext[0] += 1;
@@ -243,7 +245,7 @@ public class CipherStreamTest
     private void testWriteRead(Object cipher, CipherParameters params, boolean blocks)
         throws Exception
     {
-        byte[] data = new byte[1000];
+        byte[] data = new byte[streamSize];
         for (int i = 0; i < data.length; i++)
         {
             data[i] = (byte)(i % 255);
@@ -274,10 +276,10 @@ public class CipherStreamTest
             OutputStream cOut = createCipherOutputStream(bOut, cipher);
             if (blocks)
             {
-                int chunkSize = data.length / 8;
+                int chunkSize = Math.max(1, data.length / 8);
                 for (int i = 0; i < data.length; i += chunkSize)
                 {
-                    cOut.write(data, i, chunkSize);
+                    cOut.write(data, i, Math.min(chunkSize, data.length - i));
                 }
             }
             else
@@ -471,6 +473,17 @@ public class CipherStreamTest
     public void performTest()
         throws Exception
     {
+        int[] testSizes = new int[]{0, 1, 7, 8, 9, 15, 16, 17, 1023, 1024, 1025, 2047, 2048, 2049, 4095, 4096, 4097};
+        for (int i = 0; i < testSizes.length; i++)
+        {
+            this.streamSize = testSizes[i];
+            performTests();
+        }
+    }
+
+    private void performTests()
+        throws Exception
+    {
         testModes(new BlowfishEngine(), new BlowfishEngine(), 16);
         testModes(new DESEngine(), new DESEngine(), 8);
         testModes(new DESedeEngine(), new DESedeEngine(), 24);
@@ -498,7 +511,6 @@ public class CipherStreamTest
         testMode(new Grain128Engine(), new ParametersWithIV(new KeyParameter(new byte[16]), new byte[12]));
         testMode(new HC128Engine(), new KeyParameter(new byte[16]));
         testMode(new HC256Engine(), new ParametersWithIV(new KeyParameter(new byte[16]), new byte[16]));
-
     }
 
     private void testModes(BlockCipher cipher1, BlockCipher cipher2, int keySize)
@@ -518,7 +530,8 @@ public class CipherStreamTest
             testMode(new BufferedBlockCipher(new CFBBlockCipher(cipher1, blockSize)), withIv);
             testMode(new BufferedBlockCipher(new SICBlockCipher(cipher1)), withIv);
         }
-        if (blockSize <= 16)
+        // CTS requires at least one block
+        if (blockSize <= 16 && streamSize >= blockSize)
         {
             testMode(new CTSBlockCipher(cipher1), key);
         }