From 2dcc0649166518cb61160822b1d54bbb3784038b Mon Sep 17 00:00:00 2001 From: "Lasse R.H. Nielsen" Date: Mon, 11 Oct 2021 14:31:37 +0000 Subject: [PATCH] Add range-check to `DateTime.fromMillisecondsSinceEpoch` on VM. The current constructor allowed large numbers which overflows when multiplied by 1000 to get microseconds, which then hid the fact that the original value was out of range. Now checks directly in the `fromMillisecondsSinceEpoch` constructor, before multiplying by 1000. Fixes #46966 BUG= http://dartbug.com/46966 TEST= Regression test added to date_time_test.dart Change-Id: I4c6448666a49d51c285bd538e05e51a141b3b0b2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214641 Commit-Queue: Lasse R.H. Nielsen Reviewed-by: Nate Bosch --- sdk/lib/_internal/vm/lib/date_patch.dart | 10 +++++++- tests/corelib/date_time_test.dart | 29 ++++++++++++++++++++++++ tests/corelib_2/date_time_test.dart | 29 ++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/sdk/lib/_internal/vm/lib/date_patch.dart b/sdk/lib/_internal/vm/lib/date_patch.dart index eec2bf50ff5..249399a9e39 100644 --- a/sdk/lib/_internal/vm/lib/date_patch.dart +++ b/sdk/lib/_internal/vm/lib/date_patch.dart @@ -39,7 +39,8 @@ class DateTime { DateTime.fromMillisecondsSinceEpoch(int millisecondsSinceEpoch, {bool isUtc: false}) : this._withValue( - millisecondsSinceEpoch * Duration.microsecondsPerMillisecond, + _validateMilliseconds(millisecondsSinceEpoch) * + Duration.microsecondsPerMillisecond, isUtc: isUtc); @patch @@ -58,6 +59,13 @@ class DateTime { if (isUtc == null) throw new ArgumentError(); } + static int _validateMilliseconds(int millisecondsSinceEpoch) => + RangeError.checkValueInInterval( + millisecondsSinceEpoch, + -_maxMillisecondsSinceEpoch, + _maxMillisecondsSinceEpoch, + "millisecondsSinceEpoch"); + @patch DateTime._now() : isUtc = false, diff --git a/tests/corelib/date_time_test.dart b/tests/corelib/date_time_test.dart index e782ca54954..21859ef50ef 100644 --- a/tests/corelib/date_time_test.dart +++ b/tests/corelib/date_time_test.dart @@ -1188,6 +1188,34 @@ void testIsoString() { Expect.equals("-010000-01-01T23:49:59.989979Z", d.toIso8601String()); } +void testRegression46966() { + // See http://dartbug.com/46966 + // The constructor allowed numbers larger than 100_000_000 days + // from epoch, contrary to documentation (and web behavior). + + // Maximally allowed milliseconds on either side of epoch + // (±100 000 000 days). + var maxMilliseconds = 100000000 * 24 * 60 * 60 * 1000; + var maxDate = DateTime.fromMillisecondsSinceEpoch(maxMilliseconds); + var minDate = DateTime.fromMillisecondsSinceEpoch(-maxMilliseconds); + + // Throws if greater. + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(maxMilliseconds + 1)); + Expect.throws( + () => DateTime.fromMillisecondsSinceEpoch(-maxMilliseconds - 1)); + + // But also if much greater. + // The badMin value overflows 64-bits when multiplied by 1000. + var badMin = 0x20c49ba5e353f8; + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(badMin)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(-badMin)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(badMin + 1)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(-badMin - 1)); + var badMax = double.maxFinite.toInt(); // 2^63-1 on VM, max double on JS. + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(badMax)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(-badMax - 1)); +} + void main() { testNow(); testMillisecondsSinceEpoch(); @@ -1203,4 +1231,5 @@ void main() { testWeekday(); testToStrings(); testIsoString(); + testRegression46966(); } diff --git a/tests/corelib_2/date_time_test.dart b/tests/corelib_2/date_time_test.dart index 798fc3a490d..09bbecd3b46 100644 --- a/tests/corelib_2/date_time_test.dart +++ b/tests/corelib_2/date_time_test.dart @@ -1190,6 +1190,34 @@ void testIsoString() { Expect.equals("-010000-01-01T23:49:59.989979Z", d.toIso8601String()); } +void testRegression46966() { + // See http://dartbug.com/46966 + // The constructor allowed numbers larger than 100_000_000 days + // from epoch, contrary to documentation (and web behavior). + + // Maximally allowed milliseconds on either side of epoch + // (±100 000 000 days). + var maxMilliseconds = 100000000 * 24 * 60 * 60 * 1000; + var maxDate = DateTime.fromMillisecondsSinceEpoch(maxMilliseconds); + var minDate = DateTime.fromMillisecondsSinceEpoch(-maxMilliseconds); + + // Throws if greater. + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(maxMilliseconds + 1)); + Expect.throws( + () => DateTime.fromMillisecondsSinceEpoch(-maxMilliseconds - 1)); + + // But also if much greater. + // The badMin value overflows 64-bits when multiplied by 1000. + var badMin = 0x20c49ba5e353f8; + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(badMin)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(-badMin)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(badMin + 1)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(-badMin - 1)); + var badMax = double.maxFinite.toInt(); // 2^63-1 on VM, max double on JS. + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(badMax)); + Expect.throws(() => DateTime.fromMillisecondsSinceEpoch(-badMax - 1)); +} + void main() { testNow(); testMillisecondsSinceEpoch(); @@ -1205,4 +1233,5 @@ void main() { testWeekday(); testToStrings(); testIsoString(); + testRegression46966(); }