United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-6788196 : (porting) Bounds checks in io_util.c rely on undefined behaviour

Details
Type:
Bug
Submit Date:
2008-12-22
Status:
Resolved
Updated Date:
2011-07-20
Project Name:
JDK
Resolved Date:
2009-01-17
Component:
core-libs
OS:
linux
Sub-Component:
java.io
CPU:
x86
Priority:
P4
Resolution:
Fixed
Affected Versions:
5.0u17,7
Fixed Versions:

Related Reports
Backport:
Backport:
Backport:
Backport:

Sub Tasks

Description
FULL PRODUCT VERSION :
java version "1.6.0_0"
OpenJDK Runtime Environment (build 1.6.0_0-b12)
OpenJDK Core VM (build 14.0-b08, interpreted mode)
(built from icedtea6-ce9956fe8908)

ADDITIONAL OS VERSION INFORMATION :
All platforms

A DESCRIPTION OF THE PROBLEM :
In jdk/src/share/native/java/io/io_util.c, both readBytes and writeBytes include the following array bounds check:

  if ((off < 0) || (off > datalen) ||
      (len < 0) || ((off + len) > datalen) || ((off + len) < 0)) {
         JNU_ThrowByName(env, "java/lang/IndexOutOfBoundsException", 0);
         return -1;
     }

off, len and datalen are all signed 32-bit integers.  The result of (off + len) is undefined in C if the result overflows, which can cause the exception not to be thrown. This causes testsuite failures on 32-bit Linux PowerPC on Fedora 10.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Compile the test case and run it on 32-bit Linux PowerPC on Fedora 10.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Exception in thread "main" java.lang.IndexOutOfBoundsException
	at java.io.FileInputStream.readBytes(Native Method)
	at java.io.FileInputStream.read(FileInputStream.java:236)
	at Test.main(Test.java:5)

ACTUAL -
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException
	at java.io.FileInputStream.readBytes(Native Method)
	at java.io.FileInputStream.read(FileInputStream.java:236)
	at Test.main(Test.java:5)

Note that the thrown exception is different; I'm guessing it comes from the (*env)->SetByteArrayRegion further down readBytes.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.io.FileInputStream;
class Test {
  public static void main(String[] args) throws Exception {
    byte[] b = new byte[20];
    (new FileInputStream("/bin/ls")).read(b, 1, 0x7fffffff);
  }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
http://icedtea.classpath.org/hg/icedtea6/file/ce9956fe8908/patches/icedtea-io_util-overflow.patch

                                    

Comments
EVALUATION

The submitter is correct and this code is not portable. This bounds check has existed since at least jdk1.2. The proposed fix casts to uint32_t. An alternative is to change the check to:

--- a/src/share/native/java/io/io_util.c        Wed Dec 10 14:03:15 2008 -0800
+++ b/src/share/native/java/io/io_util.c        Tue Dec 23 20:07:06 2008 +0000
@@ -74,8 +74,7 @@ readBytes(JNIEnv *env, jobject this, jby
     }
     datalen = (*env)->GetArrayLength(env, bytes);

-    if ((off < 0) || (off > datalen) ||
-        (len < 0) || ((off + len) > datalen) || ((off + len) < 0)) {
+    if ((off < 0) || (len < 0) || (len > (datalen - off))) {
         JNU_ThrowByName(env, "java/lang/IndexOutOfBoundsException", 0);
         return -1;
     }
@@ -147,8 +146,7 @@ writeBytes(JNIEnv *env, jobject this, jb
     }
     datalen = (*env)->GetArrayLength(env, bytes);

-    if ((off < 0) || (off > datalen) ||
-        (len < 0) || ((off + len) > datalen) || ((off + len) < 0)) {
+    if ((off < 0) || (len < 0) || (len > (datalen - off))) {
         JNU_ThrowByName(env, "java/lang/IndexOutOfBoundsException", 0);
         return;
     }
                                     
2008-12-23
EVALUATION

Martin plans to push a change-set for this, see discussion at:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2008-December/000955.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-January/000967.html
                                     
2009-01-07



Hardware and Software, Engineered to Work Together