Time.parse3339 range checking and proper 'sec-frac' skip

The parse3339 JNI code doesn't properly do bounds checking on the input String.
These changes do some bounds checking to prevent a buffer underflow condition.

parse3339 should allow the fractional seconds to be optional and an arbitrary
length as specified in RFC 3339. This will scan through arbitrary precision
until it finds the timezone indicators.

Change-Id: Ie9d01d0b24163d893c58c747d37873c83b74e6c7

2 files changed, 115 insertions, 12 deletions

diff --git a/core/jni/android_text_format_Time.cpp b/core/jni/android_text_format_Time.cpp
index fde6ca6..d89a7e6 100644
--- a/core/jni/android_text_format_Time.cpp
+++ b/core/jni/android_text_format_Time.cpp
@@ -382,7 +382,7 @@ static bool check_char(JNIEnv* env, const jchar *s, int spos, jchar expected)
     jchar c = s[spos];
     if (c != expected) {
         char msg[100];
-	sprintf(msg, "Unexpected %c at pos=%d.  Expected %c.", c, spos,
+	sprintf(msg, "Unexpected character 0x%02x at pos=%d.  Expected %c.", c, spos,
 		expected);
 	jniThrowException(env, "android/util/TimeFormatException", msg);
 	return false;
@@ -483,6 +483,12 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
     int n;
     jboolean inUtc = false;
 
+    if (len < 10) {
+        jniThrowException(env, "android/util/TimeFormatException",
+                "Time input is too short; must be at least 10 characters");
+        return false;
+    }
+
     // year
     n = get_char(env, s, 0, 1000, &thrown);    
     n += get_char(env, s, 1, 100, &thrown);
@@ -510,7 +516,7 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
     if (thrown) return false;
     env->SetIntField(This, g_mdayField, n);
 
-    if (len >= 17) {
+    if (len >= 19) {
         // T
         if (!check_char(env, s, 10, 'T')) return false;
 
@@ -541,10 +547,19 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
         if (thrown) return false;
         env->SetIntField(This, g_secField, n);
 
-	// skip the '.XYZ' -- we don't care about subsecond precision.
+        // skip the '.XYZ' -- we don't care about subsecond precision.
+        int tz_index = 19;
+        if (tz_index < len && s[tz_index] == '.') {
+            do {
+                tz_index++;
+            } while (tz_index < len
+                && s[tz_index] >= '0'
+                && s[tz_index] <= '9');
+        }
+
         int offset = 0;
-	if (len >= 23) {
-	    char c = s[23];
+        if (len > tz_index) {
+            char c = s[tz_index];
 
 	    // NOTE: the offset is meant to be subtracted to get from local time
 	    // to UTC.  we therefore use 1 for '-' and -1 for '+'.
@@ -561,27 +576,34 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
 	        break;
 	    default:
 	        char msg[100];
-	        sprintf(msg, "Unexpected %c at position 19.  Expected + or -",
-			c);
+	        sprintf(msg, "Unexpected character 0x%02x at position %d.  Expected + or -",
+			c, tz_index);
 	        jniThrowException(env, "android/util/TimeFormatException", msg);
 	        return false;
 	    }
             inUtc = true;
 
 	    if (offset != 0) {
+	        if (len < tz_index + 5) {
+	            char msg[100];
+	            sprintf(msg, "Unexpected length; should be %d characters", tz_index + 5);
+	            jniThrowException(env, "android/util/TimeFormatException", msg);
+	            return false;
+	        }
+
 	        // hour
-	        n = get_char(env, s, 24, 10, &thrown);
-		n += get_char(env, s, 25, 1, &thrown);
+	        n = get_char(env, s, tz_index + 1, 10, &thrown);
+		n += get_char(env, s, tz_index + 2, 1, &thrown);
 		if (thrown) return false;
 		n *= offset;
 		hour += n;
 
 		// :
-		if (!check_char(env, s, 26, ':')) return false;
+		if (!check_char(env, s, tz_index + 3, ':')) return false;
 	    
 		// minute
-		n = get_char(env, s, 27, 10, &thrown);
-		n += get_char(env, s, 28, 1, &thrown);
+		n = get_char(env, s, tz_index + 4, 10, &thrown);
+		n += get_char(env, s, tz_index + 5, 1, &thrown);
 		if (thrown) return false;
 		n *= offset;
 		minute += n;
diff --git a/tests/AndroidTests/src/com/android/unit_tests/TimeTest.java b/tests/AndroidTests/src/com/android/unit_tests/TimeTest.java
index 110caa4..3b33a99 100644
--- a/tests/AndroidTests/src/com/android/unit_tests/TimeTest.java
+++ b/tests/AndroidTests/src/com/android/unit_tests/TimeTest.java
@@ -20,6 +20,7 @@ import android.test.suitebuilder.annotation.SmallTest;
 import android.test.suitebuilder.annotation.Suppress;
 import android.text.format.Time;
 import android.util.Log;
+import android.util.TimeFormatException;
 
 import junit.framework.TestCase;
 
@@ -354,6 +355,86 @@ public class TimeTest extends TestCase {
     }
 
     @SmallTest
+    public void testParse33390() throws Exception {
+        Time t = new Time(Time.TIMEZONE_UTC);
+
+        t.parse3339("1980-05-23");
+        if (!t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23) {
+            fail("Did not parse all-day date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != 0) {
+            fail("Did not parse timezone-offset-less date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50Z");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != 0) {
+            fail("Did not parse UTC date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50.0Z");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != 0) {
+            fail("Did not parse UTC date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50.12Z");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != 0) {
+            fail("Did not parse UTC date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50.123Z");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != 0) {
+            fail("Did not parse UTC date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50-06:00");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != -6*3600) {
+            fail("Did not parse timezone-offset date correctly");
+        }
+
+        t.parse3339("1980-05-23T09:50:50.123-06:00");
+        if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
+                t.hour != 9 || t.minute != 50 || t.second != 50 ||
+                t.gmtoff != -6*3600) {
+            fail("Did not parse timezone-offset date correctly");
+        }
+
+        try {
+            t.parse3339("1980");
+            fail("Did not throw error on truncated input length");
+        } catch (TimeFormatException e) {
+            // Successful
+        }
+
+        try {
+            t.parse3339("1980-05-23T09:50:50.123+");
+            fail("Did not throw error on truncated timezone offset");
+        } catch (TimeFormatException e1) {
+            // Successful
+        }
+
+        try {
+            t.parse3339("1980-05-23T09:50:50.123+05:0");
+            fail("Did not throw error on truncated timezone offset");
+        } catch (TimeFormatException e1) {
+            // Successful
+        }
+    }
+
+    @SmallTest
     public void testSet0() throws Exception {
         Time t = new Time(Time.TIMEZONE_UTC);
         t.set(1000L);