From 2488a51e82c40b41b3226100d18323fee5e34613 Mon Sep 17 00:00:00 2001 From: Ralph Loader Date: Thu, 16 Oct 2003 21:28:23 +0000 Subject: [PATCH] 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 --- libjava/ChangeLog | 8 + libjava/java/lang/natString.cc | 26 ++-- .../libjava.lang/String_overflow.java | 140 ++++++++++++++++++ .../libjava.lang/String_overflow.out | 1 + 4 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 libjava/testsuite/libjava.lang/String_overflow.java create mode 100644 libjava/testsuite/libjava.lang/String_overflow.out diff --git a/libjava/ChangeLog b/libjava/ChangeLog index b34b427c6d8..ef0511712b8 100644 --- a/libjava/ChangeLog +++ b/libjava/ChangeLog @@ -1,3 +1,11 @@ +2003-10-17 Ralph Loader + + * 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 * prims.cc (_Jv_NewObjectArray): Make sure byte size doesn't diff --git a/libjava/java/lang/natString.cc b/libjava/java/lang/natString.cc index c87844b0d51..8217f703995 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 00000000000..5a1a907c503 --- /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 00000000000..9766475a418 --- /dev/null +++ b/libjava/testsuite/libjava.lang/String_overflow.out @@ -0,0 +1 @@ +ok