Flutterfire: [cloud_firestore] Timestamp class miscalculates microseconds and milliseconds

Created on 13 Oct 2019  路  4Comments  路  Source: FirebaseExtended/flutterfire

To test the Timestamp class in "Timestamp.dart" file, try this:

int _kEndOfTime = 253402300800;
var maxTimestamp = Timestamp(_kEndOfTime - 1, _kBillion - 1);
Timestamp.fromMicrosecondsSinceEpoch(maxTimestamp.microsecondsSinceEpoch)

You get this error:

Invalid argument(s): Timestamp seconds out of range: 253402300800

The problem is the use of double (floating point) in calculations, which obviously results in rounding mistakes. Please note, this won't throw errors when the date is far from the Timestamp range limits, but will still miscalculate the number of microseconds! So this must be fixed as soon as possible.

I believe the code should be changed into this:

factory Timestamp.fromMillisecondsSinceEpoch(int milliseconds) {
  final int seconds = (milliseconds ~/ _kThousand);
  final int nanoseconds = (milliseconds - seconds * _kThousand) * _kMillion;
  return Timestamp(seconds, nanoseconds);
}

factory Timestamp.fromMicrosecondsSinceEpoch(int microseconds) {
  final int seconds = (microseconds ~/ _kMillion);
  final int nanoseconds = (microseconds - seconds * _kMillion) * _kThousand;
  return Timestamp(seconds, nanoseconds);
}

int get millisecondsSinceEpoch => (seconds * _kThousand + nanoseconds ~/ _kMillion);

int get microsecondsSinceEpoch => (seconds * _kMillion + nanoseconds ~/ _kThousand);
crowd cloud_firestore bug

Most helpful comment

@kroikie Yes, I hope this issue is given some attention now, since it presents potential security problems. Can you please mark this with a security label or similar, so that it is given some priority?

All 4 comments

@marcglasberg

The issue at https://github.com/flutter/flutter/issues/34929 has been closed and moved here. Future collaboration on this issue will be done here.

@kroikie Yes, I hope this issue is given some attention now, since it presents potential security problems. Can you please mark this with a security label or similar, so that it is given some priority?

Any progress about this issue?

@Ehesp Hi Elliot, this is a very serious issue which is very easy to fix. Sorry to ping you directly, but it's unbelivable that this is still standing. Should be maximum priority. Thanks.

Was this page helpful?
0 / 5 - 0 ratings