natString.cc (getChars): Fix validation of array indexes.

	* java/lang/natString.cc (getChars):
	Fix validation of array indexes.
	(getBytes, regionMatches, startsWith, valueOf): Likewise.
	* testsuite/libjava.lang/String_overflow.java: New file.
	* testsuite/libjava.lang/String_overflow.out: New file.

From-SVN: r72578

4 files changed, 165 insertions, 10 deletions

diff --git a/libjava/ChangeLog b/libjava/ChangeLog
index b34b427..ef05117 100644
--- a/libjava/ChangeLog
+++ b/libjava/ChangeLog
@@ -1,5 +1,13 @@
 2003-10-17  Ralph Loader  <rcl@ihug.co.nz>
 
+	* java/lang/natString.cc (getChars):
+	Fix validation of array indexes.
+	(getBytes, regionMatches, startsWith, valueOf): Likewise.
+	* testsuite/libjava.lang/String_overflow.java: New file.
+	* testsuite/libjava.lang/String_overflow.out: New file.
+
+2003-10-17  Ralph Loader  <rcl@ihug.co.nz>
+
 	* prims.cc (_Jv_NewObjectArray): Make sure byte size doesn't
 	overflow a jint.
 	(_Jv_NewPrimArray): Check for overflowing a jint, replacing a
diff --git a/libjava/java/lang/natString.cc b/libjava/java/lang/natString.cc
index c87844b..8217f70 100644
--- a/libjava/java/lang/natString.cc
+++ b/libjava/java/lang/natString.cc
@@ -601,7 +601,10 @@ java::lang::String::getChars(jint srcBegin, jint srcEnd,
   jint dst_length = JvGetArrayLength (dst);
   if (srcBegin < 0 || srcBegin > srcEnd || srcEnd > count)
     throw new java::lang::StringIndexOutOfBoundsException;
-  if (dstBegin < 0 || dstBegin + (srcEnd-srcBegin) > dst_length)
+  // The 2nd part of the test below is equivalent to 
+  // dstBegin + (srcEnd-srcBegin) > dst_length
+  // except that it does not overflow.
+  if (dstBegin < 0 || dstBegin > dst_length - (srcEnd-srcBegin))
     throw new ArrayIndexOutOfBoundsException;
   jchar *dPtr = elements (dst) + dstBegin;
   jchar *sPtr = JvGetStringChars (this) + srcBegin;
@@ -653,7 +656,10 @@ java::lang::String::getBytes(jint srcBegin, jint srcEnd,
   jint dst_length = JvGetArrayLength (dst);
   if (srcBegin < 0 || srcBegin > srcEnd || srcEnd > count)
     throw new java::lang::StringIndexOutOfBoundsException;
-  if (dstBegin < 0 || dstBegin + (srcEnd-srcBegin) > dst_length)
+  // The 2nd part of the test below is equivalent to 
+  // dstBegin + (srcEnd-srcBegin) > dst_length
+  // except that it does not overflow.
+  if (dstBegin < 0 || dstBegin > dst_length - (srcEnd-srcBegin))
     throw new ArrayIndexOutOfBoundsException;
   jbyte *dPtr = elements (dst) + dstBegin;
   jchar *sPtr = JvGetStringChars (this) + srcBegin;
@@ -700,9 +706,9 @@ jboolean
 java::lang::String::regionMatches (jint toffset,
 				   jstring other, jint ooffset, jint len)
 {
-  if (toffset < 0 || ooffset < 0
-      || toffset + len > count
-      || ooffset + len > other->count)
+  if (toffset < 0 || ooffset < 0 || len < 0
+      || toffset > count - len
+      || ooffset > other->count - len)
     return false;
   jchar *tptr = JvGetStringChars (this) + toffset;
   jchar *optr = JvGetStringChars (other) + ooffset;
@@ -737,9 +743,9 @@ jboolean
 java::lang::String::regionMatches (jboolean ignoreCase, jint toffset,
 				   jstring other, jint ooffset, jint len)
 {
-  if (toffset < 0 || ooffset < 0
-      || toffset + len > count
-      || ooffset + len > other->count)
+  if (toffset < 0 || ooffset < 0 || len < 0
+      || toffset > count - len
+      || ooffset > other->count - len)
     return false;
   jchar *tptr = JvGetStringChars (this) + toffset;
   jchar *optr = JvGetStringChars (other) + ooffset;
@@ -770,7 +776,7 @@ jboolean
 java::lang::String::startsWith (jstring prefix, jint toffset)
 {
   jint i = prefix->count;
-  if (toffset < 0 || toffset + i > count)
+  if (toffset < 0 || toffset > count - i)
     return false;
   jchar *xptr = JvGetStringChars (this) + toffset;
   jchar *yptr = JvGetStringChars (prefix);
@@ -1043,7 +1049,7 @@ jstring
 java::lang::String::valueOf(jcharArray data, jint offset, jint count)
 {
   jint data_length = JvGetArrayLength (data);
-  if (offset < 0 || count < 0 || offset+count > data_length)
+  if (offset < 0 || count < 0 || offset > data_length - count)
     throw new ArrayIndexOutOfBoundsException;
   jstring result = JvAllocString(count);
   jchar *sPtr = elements (data) + offset;
diff --git a/libjava/testsuite/libjava.lang/String_overflow.java b/libjava/testsuite/libjava.lang/String_overflow.java
new file mode 100644
index 0000000..5a1a907
--- /dev/null
+++ b/libjava/testsuite/libjava.lang/String_overflow.java
@@ -0,0 +1,140 @@
+class String_overflow
+{
+  static void getChars()
+  {
+    String source = "abcdefg";
+    char[] dest = new char [3];
+
+    try
+      {
+	source.getChars (0, 5,	// Source
+			 dest, (1<<31) - 1);
+	Fail ("getChars", "Should not have succeeded");
+      }
+    catch (Throwable e)
+      {
+	ExpectArrayIndex ("getChars", e);
+      }
+  }
+
+    /* How do I stop a compiler warning causing a test to fail?
+  static void getBytes()
+  {
+    String source = "abcdefg";
+    byte[] dest = new byte[3];
+
+    try
+      {
+	source.getBytes (0, 5, dest, (1<<31) - 1);
+	Fail ("getBytes", "Should not have succeeded");
+      }
+    catch (Throwable e)
+      {
+	ExpectArrayIndex ("getBytes", e);
+      }
+  }
+    */
+
+  static void regionMatches()
+  {
+    if ("abcdefg".regionMatches (4, "abcdefg", 4, -1))
+      {
+	Fail ("regionMatches", "Should not return true");
+      }
+
+    try
+      {
+	if ("abcdefg".regionMatches (4, "abcdefg", 4, (1<<31)-1))
+	  {
+	    Fail ("regionMatches (2nd)", "Should not return true");
+	  }
+      }
+    catch (Throwable e)
+      {
+	Fail ("regionMatches (2nd)", e);
+      }
+  }
+
+  static void regionMatchesCase()
+  {
+    if ("abcdefg".regionMatches (true, 4, "abcdefg", 4, -1))
+      {
+	Fail ("regionMatchesCase", "Should not return true");
+      }
+
+    try
+      {
+	if ("abcdefg".regionMatches (true, 4, "abcdefg", 4, (1<<31)-1))
+	  {
+	    Fail ("regionMatchesCase (2nd)", "Should not return true");
+	  }
+      }
+    catch (Throwable e)
+      {
+	Fail ("regionMatchesCase (2nd)", e);
+      }
+  }
+
+  static void startsWith()
+  {
+    // We make the arg pretty big to try and cause a segfault.
+    String s = new String ("abcdef");
+    StringBuffer b = new StringBuffer (1000000);
+    b.setLength (1000000);
+    String arg = new String (b);
+
+    try
+      {
+	s.startsWith (arg, (1<<31) - 1000000);
+      }
+    catch (Throwable e)
+      {
+	Fail ("startsWith", e);
+      }
+  }
+
+  static void valueOf()
+  {
+    char[] array = new char[] {'a', 'b', 'c', 'd', 'e'};
+    try
+      {
+	String.valueOf (array, 4, (1<<31)-1);
+	Fail ("valueOf", "should not succeed");
+      }
+    catch (Throwable e)
+      {
+	ExpectArrayIndex ("valueOf", e);
+      }
+  }
+
+  public static void main (String[] args) throws Throwable
+  {
+    getChars();
+    //    getBytes();
+    regionMatches();
+    regionMatchesCase();
+    startsWith();
+    valueOf();
+
+    if (tests_failed == 0)
+      System.out.println ("ok");
+  }
+
+  static void ExpectArrayIndex (String test, Throwable e)
+  {
+    if (e instanceof ArrayIndexOutOfBoundsException)
+      return;
+
+    Fail (test, e);
+  }
+
+  static void Fail (String test, Object problem)
+  {
+    ++tests_failed;
+    System.err.print (test);
+    System.err.print ('\t');
+    System.err.println (problem);
+  }
+
+  static int tests_failed;
+}
diff --git a/libjava/testsuite/libjava.lang/String_overflow.out b/libjava/testsuite/libjava.lang/String_overflow.out
new file mode 100644
index 0000000..9766475
--- /dev/null
+++ b/libjava/testsuite/libjava.lang/String_overflow.out
@@ -0,0 +1 @@
+ok