Skip to content

Commit 6c2d402

Browse files
krutonkruton
committed
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
1 parent 05ffc25 commit 6c2d402

File tree

2 files changed

+115
-12
lines changed

2 files changed

+115
-12
lines changed

‎core/jni/android_text_format_Time.cpp‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ static bool check_char(JNIEnv* env, const jchar *s, int spos, jchar expected)
382382
jchar c = s[spos];
383383
if (c != expected) {
384384
char msg[100];
385-
sprintf(msg, "Unexpected %c at pos=%d. Expected %c.", c, spos,
385+
sprintf(msg, "Unexpected character 0x%02x at pos=%d. Expected %c.", c, spos,
386386
expected);
387387
jniThrowException(env, "android/util/TimeFormatException", msg);
388388
return false;
@@ -483,6 +483,12 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
483483
int n;
484484
jboolean inUtc = false;
485485

486+
if (len < 10) {
487+
jniThrowException(env, "android/util/TimeFormatException",
488+
"Time input is too short; must be at least 10 characters");
489+
return false;
490+
}
491+
486492
// year
487493
n = get_char(env, s, 0, 1000, &thrown);
488494
n += get_char(env, s, 1, 100, &thrown);
@@ -510,7 +516,7 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
510516
if (thrown) return false;
511517
env->SetIntField(This, g_mdayField, n);
512518

513-
if (len >= 17) {
519+
if (len >= 19) {
514520
// T
515521
if (!check_char(env, s, 10, 'T')) return false;
516522

@@ -541,10 +547,19 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
541547
if (thrown) return false;
542548
env->SetIntField(This, g_secField, n);
543549

544-
// skip the '.XYZ' -- we don't care about subsecond precision.
550+
// skip the '.XYZ' -- we don't care about subsecond precision.
551+
int tz_index = 19;
552+
if (tz_index < len && s[tz_index] == '.') {
553+
do {
554+
tz_index++;
555+
} while (tz_index < len
556+
&& s[tz_index] >= '0'
557+
&& s[tz_index] <= '9');
558+
}
559+
545560
int offset = 0;
546-
if (len >= 23) {
547-
char c = s[23];
561+
if (len > tz_index) {
562+
char c = s[tz_index];
548563

549564
// NOTE: the offset is meant to be subtracted to get from local time
550565
// to UTC. we therefore use 1 for '-' and -1 for '+'.
@@ -561,27 +576,34 @@ static jboolean android_text_format_Time_parse3339(JNIEnv* env,
561576
break;
562577
default:
563578
char msg[100];
564-
sprintf(msg, "Unexpected %c at position 19. Expected + or -",
565-
c);
579+
sprintf(msg, "Unexpected character 0x%02x at position %d. Expected + or -",
580+
c, tz_index);
566581
jniThrowException(env, "android/util/TimeFormatException", msg);
567582
return false;
568583
}
569584
inUtc = true;
570585

571586
if (offset != 0) {
587+
if (len < tz_index + 5) {
588+
char msg[100];
589+
sprintf(msg, "Unexpected length; should be %d characters", tz_index + 5);
590+
jniThrowException(env, "android/util/TimeFormatException", msg);
591+
return false;
592+
}
593+
572594
// hour
573-
n = get_char(env, s, 24, 10, &thrown);
574-
n += get_char(env, s, 25, 1, &thrown);
595+
n = get_char(env, s, tz_index + 1, 10, &thrown);
596+
n += get_char(env, s, tz_index + 2, 1, &thrown);
575597
if (thrown) return false;
576598
n *= offset;
577599
hour += n;
578600

579601
// :
580-
if (!check_char(env, s, 26, ':')) return false;
602+
if (!check_char(env, s, tz_index + 3, ':')) return false;
581603

582604
// minute
583-
n = get_char(env, s, 27, 10, &thrown);
584-
n += get_char(env, s, 28, 1, &thrown);
605+
n = get_char(env, s, tz_index + 4, 10, &thrown);
606+
n += get_char(env, s, tz_index + 5, 1, &thrown);
585607
if (thrown) return false;
586608
n *= offset;
587609
minute += n;

‎tests/AndroidTests/src/com/android/unit_tests/TimeTest.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import android.test.suitebuilder.annotation.Suppress;
2121
import android.text.format.Time;
2222
import android.util.Log;
23+
import android.util.TimeFormatException;
2324

2425
import junit.framework.TestCase;
2526

@@ -353,6 +354,86 @@ public void testParse0() throws Exception {
353354
// System.out.println("t=" + t);
354355
}
355356

357+
@SmallTest
358+
public void testParse33390() throws Exception {
359+
Time t = new Time(Time.TIMEZONE_UTC);
360+
361+
t.parse3339("1980-05-23");
362+
if (!t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23) {
363+
fail("Did not parse all-day date correctly");
364+
}
365+
366+
t.parse3339("1980-05-23T09:50:50");
367+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
368+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
369+
t.gmtoff != 0) {
370+
fail("Did not parse timezone-offset-less date correctly");
371+
}
372+
373+
t.parse3339("1980-05-23T09:50:50Z");
374+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
375+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
376+
t.gmtoff != 0) {
377+
fail("Did not parse UTC date correctly");
378+
}
379+
380+
t.parse3339("1980-05-23T09:50:50.0Z");
381+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
382+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
383+
t.gmtoff != 0) {
384+
fail("Did not parse UTC date correctly");
385+
}
386+
387+
t.parse3339("1980-05-23T09:50:50.12Z");
388+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
389+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
390+
t.gmtoff != 0) {
391+
fail("Did not parse UTC date correctly");
392+
}
393+
394+
t.parse3339("1980-05-23T09:50:50.123Z");
395+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
396+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
397+
t.gmtoff != 0) {
398+
fail("Did not parse UTC date correctly");
399+
}
400+
401+
t.parse3339("1980-05-23T09:50:50-06:00");
402+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
403+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
404+
t.gmtoff != -6*3600) {
405+
fail("Did not parse timezone-offset date correctly");
406+
}
407+
408+
t.parse3339("1980-05-23T09:50:50.123-06:00");
409+
if (t.allDay || t.year != 1980 || t.month != 05 || t.monthDay != 23 ||
410+
t.hour != 9 || t.minute != 50 || t.second != 50 ||
411+
t.gmtoff != -6*3600) {
412+
fail("Did not parse timezone-offset date correctly");
413+
}
414+
415+
try {
416+
t.parse3339("1980");
417+
fail("Did not throw error on truncated input length");
418+
} catch (TimeFormatException e) {
419+
// Successful
420+
}
421+
422+
try {
423+
t.parse3339("1980-05-23T09:50:50.123+");
424+
fail("Did not throw error on truncated timezone offset");
425+
} catch (TimeFormatException e1) {
426+
// Successful
427+
}
428+
429+
try {
430+
t.parse3339("1980-05-23T09:50:50.123+05:0");
431+
fail("Did not throw error on truncated timezone offset");
432+
} catch (TimeFormatException e1) {
433+
// Successful
434+
}
435+
}
436+
356437
@SmallTest
357438
public void testSet0() throws Exception {
358439
Time t = new Time(Time.TIMEZONE_UTC);

0 commit comments

Comments
 (0)