From 69ac276d0c878c36517e8add6a4afa5fbed424d6 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Wed, 8 Jan 2025 18:22:56 -0800 Subject: [PATCH] Apply bounds to timestamps during backup import --- ts/services/backups/export.ts | 2 +- ts/services/backups/import.ts | 72 ++++++++++++------- ts/services/storageRecordOps.ts | 9 ++- ts/test-both/util/timestampLongUtils_test.ts | 76 +++++++++++++++++--- ts/util/timestamp.ts | 4 +- ts/util/timestampLongUtils.ts | 54 ++++++++++++-- 6 files changed, 172 insertions(+), 45 deletions(-) diff --git a/ts/services/backups/export.ts b/ts/services/backups/export.ts index 359c834fb..c142bfc1c 100644 --- a/ts/services/backups/export.ts +++ b/ts/services/backups/export.ts @@ -480,7 +480,7 @@ export class BackupExportStream extends Readable { : null, expireTimerVersion: attributes.expireTimerVersion, muteUntilMs: attributes.muteExpiresAt - ? getSafeLongFromTimestamp(attributes.muteExpiresAt) + ? getSafeLongFromTimestamp(attributes.muteExpiresAt, Long.MAX_VALUE) : null, markedUnread: attributes.markedUnread === true, dontNotifyForMentionsIfMuted: diff --git a/ts/services/backups/import.ts b/ts/services/backups/import.ts index ad2471a76..597e57df0 100644 --- a/ts/services/backups/import.ts +++ b/ts/services/backups/import.ts @@ -56,9 +56,10 @@ import type { } from '../../model-types.d'; import { assertDev, strictAssert } from '../../util/assert'; import { - getTimestampFromLong, - getTimestampOrUndefinedFromLong, + getCheckedTimestampFromLong, + getCheckedTimestampOrUndefinedFromLong, } from '../../util/timestampLongUtils'; +import { MAX_SAFE_DATE } from '../../util/timestamp'; import { DurationInSeconds, SECOND } from '../../util/durations'; import { calculateExpirationTimestamp } from '../../util/expirationTimer'; import { dropNull } from '../../util/dropNull'; @@ -913,7 +914,7 @@ export class BackupImportStream extends Writable { } if (contact.notRegistered) { - const timestamp = getTimestampOrUndefinedFromLong( + const timestamp = getCheckedTimestampOrUndefinedFromLong( contact.notRegistered.unregisteredTimestamp ); attrs.discoveredUnregisteredAt = timestamp || this.now; @@ -1053,7 +1054,8 @@ export class BackupImportStream extends Writable { serviceId, role: dropNull(role) ?? SignalService.Member.Role.UNKNOWN, addedByUserId: fromAciObject(Aci.fromUuidBytes(addedByUserId)), - timestamp: timestamp != null ? getTimestampFromLong(timestamp) : 0, + timestamp: + timestamp != null ? getCheckedTimestampFromLong(timestamp) : 0, }; } ), @@ -1066,7 +1068,8 @@ export class BackupImportStream extends Writable { return { aci: fromAciObject(Aci.fromUuidBytes(userId)), - timestamp: timestamp != null ? getTimestampFromLong(timestamp) : 0, + timestamp: + timestamp != null ? getCheckedTimestampFromLong(timestamp) : 0, }; } ), @@ -1082,7 +1085,8 @@ export class BackupImportStream extends Writable { return { serviceId, - timestamp: timestamp != null ? getTimestampFromLong(timestamp) : 0, + timestamp: + timestamp != null ? getCheckedTimestampFromLong(timestamp) : 0, }; }), revision: dropNull(version), @@ -1186,7 +1190,9 @@ export class BackupImportStream extends Writable { isBlockList: false, members: [], - deletedAtTimestamp: getTimestampFromLong(listItem.deletionTimestamp), + deletedAtTimestamp: getCheckedTimestampFromLong( + listItem.deletionTimestamp + ), }; } @@ -1217,7 +1223,7 @@ export class BackupImportStream extends Writable { name, restrictions: fromCallLinkRestrictionsProto(restrictions), revoked: false, - expiration: getTimestampFromLong(expirationMs) || null, + expiration: getCheckedTimestampOrUndefinedFromLong(expirationMs) ?? null, storageNeedsSync: false, }; @@ -1259,9 +1265,18 @@ export class BackupImportStream extends Writable { ? DurationInSeconds.fromMillis(chat.expirationTimerMs.toNumber()) : undefined; conversation.expireTimerVersion = chat.expireTimerVersion || 1; - conversation.muteExpiresAt = getTimestampOrUndefinedFromLong( - chat.muteUntilMs - ); + + if ( + chat.muteUntilMs != null && + chat.muteUntilMs.toNumber() >= MAX_SAFE_DATE + ) { + // Muted forever + conversation.muteExpiresAt = Number.MAX_SAFE_INTEGER; + } else { + conversation.muteExpiresAt = getCheckedTimestampOrUndefinedFromLong( + chat.muteUntilMs + ); + } conversation.markedUnread = chat.markedUnread === true; conversation.dontNotifyForMentionsIfMuted = chat.dontNotifyForMentionsIfMuted === true; @@ -1303,7 +1318,7 @@ export class BackupImportStream extends Writable { ): Promise { const { aboutMe } = options; - const timestamp = item?.dateSent?.toNumber(); + const timestamp = getCheckedTimestampOrUndefinedFromLong(item?.dateSent); const logId = `fromChatItem(${timestamp})`; strictAssert(this.ourConversation != null, `${logId}: AccountData missing`); @@ -1344,7 +1359,7 @@ export class BackupImportStream extends Writable { chatConvo.unreadCount = (chatConvo.unreadCount ?? 0) + 1; } - const expirationStartTimestamp = getTimestampOrUndefinedFromLong( + const expirationStartTimestamp = getCheckedTimestampOrUndefinedFromLong( item.expireStartDate ); const expireTimer = @@ -1566,7 +1581,7 @@ export class BackupImportStream extends Writable { status: sendStatus, updatedAt: status.timestamp != null && !status.timestamp.isZero() - ? getTimestampFromLong(status.timestamp) + ? getCheckedTimestampFromLong(status.timestamp) : undefined, }; } @@ -1584,8 +1599,12 @@ export class BackupImportStream extends Writable { }; } if (incoming) { - const receivedAtMs = incoming.dateReceived?.toNumber() || this.now; - const serverTimestamp = incoming.dateServerSent?.toNumber() || undefined; + const receivedAtMs = + getCheckedTimestampOrUndefinedFromLong(incoming.dateReceived) ?? + this.now; + const serverTimestamp = getCheckedTimestampOrUndefinedFromLong( + incoming.dateServerSent + ); const unidentifiedDeliveryReceived = incoming.sealedSender === true; @@ -1704,7 +1723,7 @@ export class BackupImportStream extends Writable { url, title: dropNull(preview.title), description: dropNull(preview.description), - date: getTimestampFromLong(preview.date), + date: getCheckedTimestampFromLong(preview.date), image: preview.image ? convertFilePointerToAttachment(preview.image) : undefined, @@ -1749,7 +1768,7 @@ export class BackupImportStream extends Writable { 'Edit history has non-standard messages' ); - const timestamp = getTimestampFromLong(rev.dateSent); + const timestamp = getCheckedTimestampFromLong(rev.dateSent); const { patch: { @@ -1808,7 +1827,9 @@ export class BackupImportStream extends Writable { strictAssert(authorConvo !== undefined, 'author conversation not found'); return { - id: getTimestampFromLong(quote.targetSentTimestamp) || null, + id: + getCheckedTimestampOrUndefinedFromLong(quote.targetSentTimestamp) ?? + null, referencedMessageNotFound: quote.targetSentTimestamp == null, authorAci: isAciString(authorConvo.serviceId) ? authorConvo.serviceId @@ -1888,8 +1909,8 @@ export class BackupImportStream extends Writable { return { emoji, fromId: authorConvo.id, - targetTimestamp: getTimestampFromLong(sentTimestamp), - timestamp: getTimestampFromLong(sentTimestamp), + targetTimestamp: getCheckedTimestampFromLong(sentTimestamp), + timestamp: getCheckedTimestampFromLong(sentTimestamp), }; }); } @@ -2300,8 +2321,9 @@ export class BackupImportStream extends Writable { : null, peerId: groupId, direction: isRingerMe ? CallDirection.Outgoing : CallDirection.Incoming, - timestamp: startedCallTimestamp.toNumber(), - endedTimestamp: getTimestampFromLong(endedCallTimestamp) || null, + timestamp: getCheckedTimestampFromLong(startedCallTimestamp), + endedTimestamp: + getCheckedTimestampOrUndefinedFromLong(endedCallTimestamp) ?? null, }; await this.saveCallHistory(callHistory); @@ -2359,7 +2381,7 @@ export class BackupImportStream extends Writable { startedById: null, peerId, direction, - timestamp: startedCallTimestamp.toNumber(), + timestamp: getCheckedTimestampFromLong(startedCallTimestamp), endedTimestamp: null, }; @@ -3128,7 +3150,7 @@ export class BackupImportStream extends Writable { mode: CallMode.Adhoc, type: CallType.Adhoc, direction: CallDirection.Unknown, - timestamp: callTimestamp.toNumber(), + timestamp: getCheckedTimestampFromLong(callTimestamp), status: fromAdHocCallStateProto(state), endedTimestamp: null, }; diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 1e4697058..5db935168 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -251,7 +251,8 @@ export async function toContactRecord( contactRecord.archived = Boolean(conversation.get('isArchived')); contactRecord.markedUnread = Boolean(conversation.get('markedUnread')); contactRecord.mutedUntilTimestamp = getSafeLongFromTimestamp( - conversation.get('muteExpiresAt') + conversation.get('muteExpiresAt'), + Long.MAX_VALUE ); if (conversation.get('hideStory') !== undefined) { contactRecord.hideStory = Boolean(conversation.get('hideStory')); @@ -505,7 +506,8 @@ export function toGroupV1Record( groupV1Record.archived = Boolean(conversation.get('isArchived')); groupV1Record.markedUnread = Boolean(conversation.get('markedUnread')); groupV1Record.mutedUntilTimestamp = getSafeLongFromTimestamp( - conversation.get('muteExpiresAt') + conversation.get('muteExpiresAt'), + Long.MAX_VALUE ); applyUnknownFields(groupV1Record, conversation); @@ -527,7 +529,8 @@ export function toGroupV2Record( groupV2Record.archived = Boolean(conversation.get('isArchived')); groupV2Record.markedUnread = Boolean(conversation.get('markedUnread')); groupV2Record.mutedUntilTimestamp = getSafeLongFromTimestamp( - conversation.get('muteExpiresAt') + conversation.get('muteExpiresAt'), + Long.MAX_VALUE ); groupV2Record.dontNotifyForMentionsIfMuted = Boolean( conversation.get('dontNotifyForMentionsIfMuted') diff --git a/ts/test-both/util/timestampLongUtils_test.ts b/ts/test-both/util/timestampLongUtils_test.ts index ab1d36e62..f3d33c9c5 100644 --- a/ts/test-both/util/timestampLongUtils_test.ts +++ b/ts/test-both/util/timestampLongUtils_test.ts @@ -8,7 +8,10 @@ import { getSafeLongFromTimestamp, getTimestampFromLong, getTimestampOrUndefinedFromLong, + getCheckedTimestampFromLong, + getCheckedTimestampOrUndefinedFromLong, } from '../../util/timestampLongUtils'; +import { MAX_SAFE_DATE } from '../../util/timestamp'; describe('getSafeLongFromTimestamp', () => { it('returns zero when passed undefined', () => { @@ -21,22 +24,38 @@ describe('getSafeLongFromTimestamp', () => { assert.strictEqual(getSafeLongFromTimestamp(-456).toString(), '-456'); }); - it('returns Long.MAX_VALUE when passed Infinity', () => { - assert(getSafeLongFromTimestamp(Infinity).equals(Long.MAX_VALUE)); + it('returns MAX_SAFE_DATE when passed Infinity', () => { + assert.strictEqual( + getSafeLongFromTimestamp(Infinity).toNumber(), + MAX_SAFE_DATE + ); }); - it("returns Long.MAX_VALUE when passed very large numbers, outside of JavaScript's safely representable range", () => { - assert.equal(getSafeLongFromTimestamp(Number.MAX_VALUE), Long.MAX_VALUE); + it('returns Long.MAX_VALUE when passed Infinity and overriden', () => { + assert( + getSafeLongFromTimestamp(Infinity, Long.MAX_VALUE).equals(Long.MAX_VALUE) + ); + }); + + it("returns MAX_SAFE_DATE when passed very large numbers, outside of JavaScript's safely representable range", () => { + assert.strictEqual( + getSafeLongFromTimestamp(Number.MAX_VALUE).toNumber(), + MAX_SAFE_DATE + ); }); }); describe('getTimestampFromLong', () => { + it('returns zero when passed negative Long', () => { + assert.equal(getTimestampFromLong(Long.fromNumber(-1)), 0); + }); + it('returns zero when passed 0 Long', () => { assert.equal(getTimestampFromLong(Long.fromNumber(0)), 0); }); - it('returns Number.MAX_SAFE_INTEGER when passed Long.MAX_VALUE', () => { - assert.equal(getTimestampFromLong(Long.MAX_VALUE), Number.MAX_SAFE_INTEGER); + it('returns MAX_SAFE_DATE when passed Long.MAX_VALUE', () => { + assert.equal(getTimestampFromLong(Long.MAX_VALUE), MAX_SAFE_DATE); }); it('returns a normal number', () => { @@ -48,6 +67,24 @@ describe('getTimestampFromLong', () => { }); }); +describe('getCheckedTimestampFromLong', () => { + it('throws on absent Long', () => { + assert.throws(() => getCheckedTimestampFromLong(null)); + }); + + it('throws on negative Long', () => { + assert.throws(() => getCheckedTimestampFromLong(Long.fromNumber(-1))); + }); + + it('throws on Long.MAX_VALUE', () => { + assert.throws(() => getCheckedTimestampFromLong(Long.MAX_VALUE)); + }); + + it('does not throw otherwise', () => { + assert.equal(getCheckedTimestampFromLong(Long.fromNumber(16)), 16); + }); +}); + describe('getTimestampOrUndefinedFromLong', () => { it('returns undefined when passed 0 Long', () => { assert.equal( @@ -56,10 +93,10 @@ describe('getTimestampOrUndefinedFromLong', () => { ); }); - it('returns Number.MAX_SAFE_INTEGER when passed Long.MAX_VALUE', () => { + it('returns MAX_SAFE_DATE when passed Long.MAX_VALUE', () => { assert.equal( getTimestampOrUndefinedFromLong(Long.MAX_VALUE), - Number.MAX_SAFE_INTEGER + MAX_SAFE_DATE ); }); @@ -71,3 +108,26 @@ describe('getTimestampOrUndefinedFromLong', () => { assert.equal(getTimestampOrUndefinedFromLong(null), undefined); }); }); + +describe('getCheckedTimestampOrUndefinedFromLong', () => { + it('throws on negative Long', () => { + assert.throws(() => + getCheckedTimestampOrUndefinedFromLong(Long.fromNumber(-1)) + ); + }); + + it('returns undefined on absent Long', () => { + assert.equal(getCheckedTimestampOrUndefinedFromLong(null), undefined); + }); + + it('returns undefined on zero Long', () => { + assert.equal(getCheckedTimestampOrUndefinedFromLong(Long.ZERO), undefined); + }); + + it('returns a normal number', () => { + assert.equal( + getCheckedTimestampOrUndefinedFromLong(Long.fromNumber(16)), + 16 + ); + }); +}); diff --git a/ts/util/timestamp.ts b/ts/util/timestamp.ts index a3603129a..e1629f1a0 100644 --- a/ts/util/timestamp.ts +++ b/ts/util/timestamp.ts @@ -207,8 +207,8 @@ export function formatDate( }); } -const MAX_SAFE_DATE = 8640000000000000; -const MIN_SAFE_DATE = -8640000000000000; +export const MAX_SAFE_DATE = 8640000000000000; +export const MIN_SAFE_DATE = -8640000000000000; export function toBoundedDate(timestamp: number): Date { return new Date(Math.max(MIN_SAFE_DATE, Math.min(timestamp, MAX_SAFE_DATE))); diff --git a/ts/util/timestampLongUtils.ts b/ts/util/timestampLongUtils.ts index 2af5ba15b..5196b200f 100644 --- a/ts/util/timestampLongUtils.ts +++ b/ts/util/timestampLongUtils.ts @@ -3,23 +3,55 @@ import Long from 'long'; -export function getSafeLongFromTimestamp(timestamp = 0): Long { - if (timestamp >= Number.MAX_SAFE_INTEGER) { - return Long.MAX_VALUE; +import { MAX_SAFE_DATE } from './timestamp'; + +export function getSafeLongFromTimestamp( + timestamp = 0, + maxValue: Long | number = MAX_SAFE_DATE +): Long { + if (timestamp >= MAX_SAFE_DATE) { + if (typeof maxValue === 'number') { + return Long.fromNumber(maxValue); + } + return maxValue; } return Long.fromNumber(timestamp); } export function getTimestampFromLong(value?: Long | null): number { - if (!value) { + if (!value || value.isNegative()) { return 0; } const num = value.toNumber(); - if (num >= Number.MAX_SAFE_INTEGER) { - return Number.MAX_SAFE_INTEGER; + if (num > MAX_SAFE_DATE) { + return MAX_SAFE_DATE; + } + + return num; +} + +export class InvalidTimestampError extends Error { + constructor(message: string) { + super(`InvalidTimestampError: ${message}`); + } +} + +export function getCheckedTimestampFromLong(value?: Long | null): number { + if (value == null) { + throw new InvalidTimestampError('No number'); + } + + const num = value.toNumber(); + + if (num < 0) { + throw new InvalidTimestampError('Underflow'); + } + + if (num > MAX_SAFE_DATE) { + throw new InvalidTimestampError('Overflow'); } return num; @@ -34,3 +66,13 @@ export function getTimestampOrUndefinedFromLong( return getTimestampFromLong(value); } + +export function getCheckedTimestampOrUndefinedFromLong( + value?: Long | null +): number | undefined { + if (!value || value.isZero()) { + return undefined; + } + + return getCheckedTimestampFromLong(value); +}