Bug 24191 - Uninitialized output buffer in javax.crypto.CipherOutputStream produces NullPointerException
Summary: Uninitialized output buffer in javax.crypto.CipherOutputStream produces NullP...
Status: RESOLVED FIXED
Alias: None
Product: classpath
Classification: Unclassified
Component: classpath (show other bugs)
Version: 0.18
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-04 14:33 UTC by Nico R.
Modified: 2007-02-01 01:25 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-10-18 20:11:24


Attachments
CipherOutStream patch (596 bytes, patch)
2006-05-31 20:17 UTC, Matt Wringe
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nico R. 2005-10-04 14:33:04 UTC
In javax.crypto.CipherOutputStream, the field outBuffer is never initialized. When process() is called, it calls out.write(outBuffer), which is equivalent to out.write(null). This produces a NullPointerException, when the buffer length is queried (b.length in java.io.OutputStream, I'd say).

The outBuffer field should be initialized to a large enough buffer that can hold the output of cipher.update(...), I think.
Comment 1 Tom Tromey 2005-10-18 20:11:24 UTC
Confirmed.
This looks simple to fix; the buffer should be allocated
using the length given by Cipher.getOutputSize.
Comment 2 Matt Wringe 2006-05-31 20:17:55 UTC
Created attachment 11560 [details]
CipherOutStream patch
Comment 3 Matt Wringe 2006-05-31 20:19:05 UTC
I have attached a patch that fixes this NullPointerException, outBuffer is now allocated using the Cipher.getOutputSize length.

Once you get past this NullPointerException, there is also another issue with blocks not being encrypted in the correct order. The attached patch also fixes this issue.
Comment 4 Rafael Teixeira 2007-01-31 20:29:42 UTC
The previous patch is outdated, now the source for this class is a lot slimmer. but still contains a problem:

public void write(byte[] buf, int off, int len)

tries to write bytes to the internal stream returned from the cipher.update call always, but it is valid for the cipher to return null (the ones I need from BouncyCastle do that).

Here is how I corrected it:

       /**
        * Write a portion of a byte array to the output stream.
        * 
        * @param buf The next bytes.
        * @param off The offset in the byte array to start.
        * @param len The number of bytes to write.
        * @throws IOException If an I/O error occurs, or if the underlying cipher is
        *           not in the correct state to transform data.
        */
       public void write(byte[] buf, int off, int len)
               throws IOException
       {
               byte[] ciphered = cipher.update(buf, off, len);
               if (ciphered != null)
                       out.write(ciphered);
       }


Formatting follows my preferences that is why I didn't cook a patch.
Comment 5 cvs-commit@developer.classpath.org 2007-02-01 01:24:39 UTC
Subject: Bug 24191

CVSROOT:	/cvsroot/classpath
Module name:	classpath
Changes by:	Casey Marshall <rsdio>	07/02/01 01:24:20

Modified files:
	javax/crypto   : CipherOutputStream.java 
	.              : ChangeLog 

Log message:
	2007-01-31  Casey Marshall  <csm@gnu.org>
	
		Fixes PR classpath/24191.
		Fix suggested by Rafael Teixeira <monoman@gmail.com>.
		* javax/crypto/CipherOutputStream.java (write): check return value
		of `update' for null.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/classpath/javax/crypto/CipherOutputStream.java?cvsroot=classpath&r1=1.3&r2=1.4
http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&r1=1.9080&r2=1.9081



Comment 6 Casey Marshall 2007-02-01 01:25:09 UTC
I've checked in a change similar to the one in comment #4.

I think this is fixed now.