OSDN Git Service

Bug 1844104: Fix buffer overwrite bugs in CharsetEncoderICU and CharsetDecoderICU.
authorMihai Preda <preda@google.com>
Mon, 25 May 2009 20:00:34 +0000 (22:00 +0200)
committerMihai Preda <preda@google.com>
Tue, 26 May 2009 13:29:52 +0000 (15:29 +0200)
And add unit test.

libcore/icu/src/main/java/com/ibm/icu4jni/charset/CharsetDecoderICU.java
libcore/icu/src/main/java/com/ibm/icu4jni/charset/CharsetEncoderICU.java
libcore/nio_char/src/test/java/tests/api/java/nio/charset/AllTests.java
libcore/nio_char/src/test/java/tests/api/java/nio/charset/CharsetEncoderDecoderBufferTest.java [new file with mode: 0644]

index 3b9bf86..206f0c8 100644 (file)
@@ -49,6 +49,11 @@ public final class CharsetDecoderICU extends CharsetDecoder{
     
     private  byte[] input = null;
     private  char[] output= null;
+
+    // BEGIN android-added
+    private byte[] allocatedInput = null;
+    private char[] allocatedOutput = null;
+    // END android-added
     
     // These instance variables are
     // always assigned in the methods
@@ -286,9 +291,12 @@ public final class CharsetDecoderICU extends CharsetDecoder{
             return out.position();
         }else{
             outEnd = out.remaining();
-            if(output==null || (outEnd > output.length)){
-                output = new char[outEnd];
+            // BEGIN android-added
+            if (allocatedOutput == null || (outEnd > allocatedOutput.length)) {
+                allocatedOutput = new char[outEnd];
             }
+            output = allocatedOutput;
+            // END android-added
             //since the new 
             // buffer start position 
             // is 0
@@ -303,9 +311,12 @@ public final class CharsetDecoderICU extends CharsetDecoder{
             return in.position()+savedInputHeldLen;/*exclude the number fo bytes held in previous conversion*/
         }else{
             inEnd = in.remaining();
-            if(input==null|| (inEnd > input.length)){ 
-                input = new byte[inEnd];
+            // BEGIN android-added
+            if (allocatedInput == null || (inEnd > allocatedInput.length)) {
+                allocatedInput = new byte[inEnd];
             }
+            input = allocatedInput;
+            // END android-added
             // save the current position
             int pos = in.position();
             in.get(input,0,inEnd);
@@ -324,6 +335,10 @@ public final class CharsetDecoderICU extends CharsetDecoder{
         }else{
             out.put(output,0,data[OUTPUT_OFFSET]);
         }
+        // BEGIN android-added
+        // release reference to output array, which may not be ours
+        output = null;
+        // END android-added
     }
     private final void setPosition(ByteBuffer in){
 
@@ -338,5 +353,9 @@ public final class CharsetDecoderICU extends CharsetDecoder{
             savedInputHeldLen = data[INPUT_HELD];
             in.position(in.position() - savedInputHeldLen);
         }       
+        // BEGIN android-added
+        // release reference to input array, which may not be ours
+        input = null;
+        // END android-added
     }
 }
index 51d67ac..ec169f4 100644 (file)
@@ -46,6 +46,11 @@ public final class CharsetEncoderICU extends CharsetEncoder {
     private char[] input = null;
     private byte[] output = null;
 
+    // BEGIN android-added
+    private char[] allocatedInput = null;
+    private byte[] allocatedOutput = null;
+    // END android-added
+
     // These instance variables are
     // always assigned in the methods
     // before being used. This class
@@ -197,10 +202,6 @@ public final class CharsetEncoderICU extends CharsetEncoder {
         data[INVALID_CHARS] = 0;
         data[INPUT_HELD] = 0;
         savedInputHeldLen = 0;
-        // BEGIN android-added
-        output = null;
-        input = null;
-        // END android-added
     }
 
     /**
@@ -332,9 +333,12 @@ public final class CharsetEncoderICU extends CharsetEncoder {
             return out.position();
         }else{
             outEnd = out.remaining();
-            if(output==null || (outEnd > output.length)){
-                output = new byte[outEnd];
+            // BEGIN android-added
+            if (allocatedOutput == null || (outEnd > allocatedOutput.length)) {
+                allocatedOutput = new byte[outEnd];
             }
+            output = allocatedOutput;
+            // END android-added
             //since the new 
             // buffer start position 
             // is 0
@@ -349,9 +353,12 @@ public final class CharsetEncoderICU extends CharsetEncoder {
             return in.position()+savedInputHeldLen;/*exclude the number fo bytes held in previous conversion*/
         }else{
             inEnd = in.remaining();
-            if(input==null|| (inEnd > input.length)){ 
-                input = new char[inEnd];
+            // BEGIN android-added
+            if (allocatedInput == null || (inEnd > allocatedInput.length)) {
+                allocatedInput = new char[inEnd];
             }
+            input = allocatedInput;
+            // END android-added
             // save the current position
             int pos = in.position();
             in.get(input,0,inEnd);
@@ -375,6 +382,10 @@ public final class CharsetEncoderICU extends CharsetEncoder {
         } else {
             out.put(output, 0, data[OUTPUT_OFFSET]);
         }
+        // BEGIN android-added
+        // release reference to output array, which may not be ours
+        output = null;
+        // END android-added
     }
     private final void setPosition(CharBuffer in){
 
@@ -408,5 +419,10 @@ public final class CharsetEncoderICU extends CharsetEncoder {
             in.position(in.position() - savedInputHeldLen);
         }     
 // END android-added
+
+        // BEGIN android-added
+        // release reference to input array, which may not be ours
+        input = null;
+        // END android-added
     }
 }
index 35e7129..f318960 100644 (file)
@@ -112,6 +112,8 @@ public class AllTests {
         suite.addTestSuite(Charset_macintosh.class);
         suite.addTestSuite(Charset_GSM0338.class);
 
+        suite.addTestSuite(CharsetEncoderDecoderBufferTest.class);
+
         return suite;
     }
 }
diff --git a/libcore/nio_char/src/test/java/tests/api/java/nio/charset/CharsetEncoderDecoderBufferTest.java b/libcore/nio_char/src/test/java/tests/api/java/nio/charset/CharsetEncoderDecoderBufferTest.java
new file mode 100644 (file)
index 0000000..5e4c3d0
--- /dev/null
@@ -0,0 +1,182 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package tests.api.java.nio.charset;
+
+import junit.framework.TestCase;
+
+import dalvik.annotation.TestTargetClass;
+import dalvik.annotation.TestTargets;
+import dalvik.annotation.TestTargetNew;
+import dalvik.annotation.TestLevel;
+
+import java.nio.charset.Charset;
+import java.nio.charset.CharsetDecoder;
+import java.nio.charset.CharsetEncoder;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+
+
+/* See bug 1844104.
+ * Checks for ICU encoder/decoder buffer corruption.
+ */
+@TestTargetClass(CharsetDecoder.class)
+public class CharsetEncoderDecoderBufferTest extends TestCase {
+
+    /* Checks for a buffer corruption that happens in ICU
+     * (CharsetDecoderICU) when a decode operation
+     * is done first with an out-buffer with hasArray()==true, and next with an out-buffer with
+     * hasArray()==false. In that situation ICU may overwrite the first out-buffer.
+     */
+    @TestTargets({
+        @TestTargetNew(
+            level = TestLevel.COMPLETE,
+            notes = "",
+            method = "decode",
+            args = {}
+        )
+    })
+    public void testDecoderOutputBuffer() {
+        CharsetDecoder decoder = Charset.forName("UTF-8").newDecoder();
+
+        char[] cBuf = new char[10];
+        CharBuffer out = CharBuffer.wrap(cBuf);
+        assertTrue(out.hasArray());
+        decoder.decode(ByteBuffer.wrap(new byte[]{(byte)'a', (byte)'b', (byte)'c', (byte)'d'}),
+                       out, false);
+
+        assertEquals("abcd", new String(cBuf, 0, 4));
+        assertEquals(0, cBuf[4]);
+        assertEquals(0, cBuf[5]);
+
+        byte[] bBuf = new byte[10];
+        out = ByteBuffer.wrap(bBuf).asCharBuffer();
+        assertFalse(out.hasArray());
+        decoder.decode(ByteBuffer.wrap(new byte[]{(byte)'x'}), out, true);
+
+        assertEquals('x', bBuf[1]);
+        assertEquals(0, bBuf[3]);
+
+        // check if the first buffer was corrupted by the second decode
+        assertEquals("abcd", new String(cBuf, 0, 4));
+        assertEquals(0, cBuf[4]);
+        assertEquals(0, cBuf[5]);
+    }
+
+    /* Checks for a buffer corruption that happens in ICU
+     * (CharsetDecoderICU) when a decode operation
+     * is done first with an in-buffer with hasArray()==true, and next with an in-buffer with
+     * hasArray()==false. In that situation ICU may overwrite the array of the first in-buffer.
+     */
+    @TestTargets({
+        @TestTargetNew(
+            level = TestLevel.COMPLETE,
+            notes = "",
+            method = "decode",
+            args = {}
+        )
+    })
+    public void testDecoderInputBuffer() {
+        CharsetDecoder decoder = Charset.forName("UTF-8").newDecoder();
+        CharBuffer out = CharBuffer.wrap(new char[10]);
+
+        byte[] inArray = {(byte)'a', (byte)'b'};
+        ByteBuffer inWithArray = ByteBuffer.wrap(inArray);
+        assertTrue(inWithArray.hasArray());
+        decoder.decode(inWithArray, out, false);
+        assertEquals('a', inArray[0]);
+        assertEquals('b', inArray[1]);
+
+        ByteBuffer inWithoutArray = ByteBuffer.allocateDirect(1);
+        inWithoutArray.put(0, (byte)'x');
+        assertFalse(inWithoutArray.hasArray());
+        decoder.decode(inWithoutArray, out, true);
+
+        // check whether the first buffer was corrupted by the second decode
+        assertEquals('a', inArray[0]);
+        assertEquals('b', inArray[1]);
+    }
+
+    /* Checks for a buffer corruption that happens in ICU
+     * (CharsetEncoderICU) when an encode operation
+     * is done first with an out-buffer with hasArray()==true, and next with an out-buffer with
+     * hasArray()==false. In that situation ICU may overwrite the first out-buffer.
+     */
+    @TestTargets({
+        @TestTargetNew(
+            level = TestLevel.COMPLETE,
+            notes = "",
+            method = "encode",
+            args = {}
+        )
+    })
+    public void testEncoderOutputBuffer() {
+        CharsetEncoder encoder = Charset.forName("UTF-8").newEncoder();
+
+        byte[] buffer = new byte[10];
+        ByteBuffer out = ByteBuffer.wrap(buffer);
+
+        assertTrue(out.hasArray());
+        encoder.encode(CharBuffer.wrap("ab"), out, false);
+
+        assertEquals('a', buffer[0]);
+        assertEquals('b', buffer[1]);
+        assertEquals(0, buffer[2]);
+
+        out = ByteBuffer.allocateDirect(10);
+        assertFalse(out.hasArray());
+        encoder.encode(CharBuffer.wrap("x"), out, true);
+
+        // check whether the second decode corrupted the first buffer
+        assertEquals('a', buffer[0]);
+        assertEquals('b', buffer[1]);
+        assertEquals(0, buffer[2]);
+    }
+
+    /* Checks for a buffer corruption that happens in ICU
+     * (CharsetEncoderICU) when an encode operation
+     * is done first with an in-buffer with hasArray()==true, and next with an in-buffer with
+     * hasArray()==false. In that situation ICU may overwrite the array of the first in-buffer.
+     */
+    @TestTargets({
+        @TestTargetNew(
+            level = TestLevel.COMPLETE,
+            notes = "",
+            method = "encode",
+            args = {}
+        )
+    })
+    public void testEncoderInputBuffer() {
+        CharsetEncoder encoder = Charset.forName("UTF-8").newEncoder();
+        ByteBuffer out = ByteBuffer.wrap(new byte[10]);
+
+        char[] inArray = {'a', 'b'};
+        CharBuffer inWithArray = CharBuffer.wrap(inArray);
+        assertTrue(inWithArray.hasArray());
+        encoder.encode(inWithArray, out, false);
+
+        assertEquals('a', inArray[0]);
+        assertEquals('b', inArray[1]);
+
+        CharBuffer inWithoutArray = CharBuffer.wrap("x");
+        assertFalse(inWithoutArray.hasArray());
+        encoder.encode(inWithoutArray, out, true);
+
+        // check whether the second decode corrupted the first buffer
+        assertEquals('a', inArray[0]);
+        assertEquals('b', inArray[1]);
+    }
+}