From 69f180a5ecbb0d36cc5c7f84f20d117d21b78949 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Fri, 12 Jan 2018 11:39:10 +0000 Subject: [PATCH] Fix some potential integer overflows for expiration time In a number of locations in the code, there were conversions of message expiration times from seconds to milliseconds, and then assigned to `long` contexts. However these conversions were being done as integer multiplication rather than long multiplication, meaning that there was a potential for overflows. Specifically, the maximum value that could be represented before overflowing was (2^31 / 1000 / 60 / 60 / 24) days = 24.8 days (< 1 month). Luckily the current allowed timeouts are all less than that value, but this fix would remove the artificial restriction, effectively allowing values of 1000x greater (68 years), at least for android. Related #5775 Closes #7338 --- .../securesms/ConversationActivity.java | 6 +++--- .../securesms/jobs/PushDecryptJob.java | 14 +++++++------- .../notifications/AndroidAutoReplyReceiver.java | 2 +- .../notifications/RemoteReplyReceiver.java | 2 +- .../securesms/service/QuickResponseService.java | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/org/thoughtcrime/securesms/ConversationActivity.java b/src/org/thoughtcrime/securesms/ConversationActivity.java index ce9e590e79..3c24057086 100644 --- a/src/org/thoughtcrime/securesms/ConversationActivity.java +++ b/src/org/thoughtcrime/securesms/ConversationActivity.java @@ -589,7 +589,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity @Override protected Void doInBackground(Void... params) { DatabaseFactory.getRecipientDatabase(ConversationActivity.this).setExpireMessages(recipient, expirationTime); - OutgoingExpirationUpdateMessage outgoingMessage = new OutgoingExpirationUpdateMessage(getRecipient(), System.currentTimeMillis(), expirationTime * 1000); + OutgoingExpirationUpdateMessage outgoingMessage = new OutgoingExpirationUpdateMessage(getRecipient(), System.currentTimeMillis(), expirationTime * 1000L); MessageSender.send(ConversationActivity.this, outgoingMessage, threadId, false, null); return null; @@ -1627,7 +1627,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity boolean forceSms = sendButton.isManualSelection() && sendButton.getSelectedTransport().isSms(); int subscriptionId = sendButton.getSelectedTransport().getSimSubscriptionId().or(-1); - long expiresIn = recipient.getExpireMessages() * 1000; + long expiresIn = recipient.getExpireMessages() * 1000L; boolean initiating = threadId == -1; Log.w(TAG, "isManual Selection: " + sendButton.isManualSelection()); @@ -1842,7 +1842,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity public void onSuccess(final @NonNull Pair result) { boolean forceSms = sendButton.isManualSelection() && sendButton.getSelectedTransport().isSms(); int subscriptionId = sendButton.getSelectedTransport().getSimSubscriptionId().or(-1); - long expiresIn = recipient.getExpireMessages() * 1000; + long expiresIn = recipient.getExpireMessages() * 1000L; boolean initiating = threadId == -1; AudioSlide audioSlide = new AudioSlide(ConversationActivity.this, result.first, result.second, MediaUtil.AUDIO_AAC, true); SlideDeck slideDeck = new SlideDeck(); diff --git a/src/org/thoughtcrime/securesms/jobs/PushDecryptJob.java b/src/org/thoughtcrime/securesms/jobs/PushDecryptJob.java index 869b9cccfa..cd850ae38c 100644 --- a/src/org/thoughtcrime/securesms/jobs/PushDecryptJob.java +++ b/src/org/thoughtcrime/securesms/jobs/PushDecryptJob.java @@ -395,7 +395,7 @@ public class PushDecryptJob extends ContextJob { Recipient recipient = getMessageDestination(envelope, message); IncomingMediaMessage mediaMessage = new IncomingMediaMessage(Address.fromExternal(context, envelope.getSource()), message.getTimestamp(), -1, - message.getExpiresInSeconds() * 1000, true, + message.getExpiresInSeconds() * 1000L, true, Optional.fromNullable(envelope.getRelay()), Optional.absent(), message.getGroupInfo(), Optional.absent()); @@ -519,7 +519,7 @@ public class PushDecryptJob extends ContextJob { Recipient recipient = getMessageDestination(envelope, message); IncomingMediaMessage mediaMessage = new IncomingMediaMessage(Address.fromExternal(context, envelope.getSource()), message.getTimestamp(), -1, - message.getExpiresInSeconds() * 1000, false, + message.getExpiresInSeconds() * 1000L, false, Optional.fromNullable(envelope.getRelay()), message.getBody(), message.getGroupInfo(), @@ -556,7 +556,7 @@ public class PushDecryptJob extends ContextJob { OutgoingExpirationUpdateMessage expirationUpdateMessage = new OutgoingExpirationUpdateMessage(recipient, message.getTimestamp(), - message.getMessage().getExpiresInSeconds() * 1000); + message.getMessage().getExpiresInSeconds() * 1000L); long threadId = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipient); long messageId = database.insertMessageOutbox(expirationUpdateMessage, threadId, false, null); @@ -576,7 +576,7 @@ public class PushDecryptJob extends ContextJob { OutgoingMediaMessage mediaMessage = new OutgoingMediaMessage(recipients, message.getMessage().getBody().orNull(), PointerAttachment.forPointers(message.getMessage().getAttachments()), message.getTimestamp(), -1, - message.getMessage().getExpiresInSeconds() * 1000, + message.getMessage().getExpiresInSeconds() * 1000L, ThreadDatabase.DistributionTypes.DEFAULT); mediaMessage = new OutgoingSecureMediaMessage(mediaMessage); @@ -602,7 +602,7 @@ public class PushDecryptJob extends ContextJob { .getExpiringMessageManager() .scheduleDeletion(messageId, true, message.getExpirationStartTimestamp(), - message.getMessage().getExpiresInSeconds() * 1000); + message.getMessage().getExpiresInSeconds() * 1000L); } return threadId; @@ -630,7 +630,7 @@ public class PushDecryptJob extends ContextJob { envelope.getSourceDevice(), message.getTimestamp(), body, message.getGroupInfo(), - message.getExpiresInSeconds() * 1000); + message.getExpiresInSeconds() * 1000L); textMessage = new IncomingEncryptedMessage(textMessage, body); Optional insertResult = database.insertMessageInbox(textMessage); @@ -652,7 +652,7 @@ public class PushDecryptJob extends ContextJob { Recipient recipient = getSyncMessageDestination(message); String body = message.getMessage().getBody().or(""); - long expiresInMillis = message.getMessage().getExpiresInSeconds() * 1000; + long expiresInMillis = message.getMessage().getExpiresInSeconds() * 1000L; if (recipient.getExpireMessages() != message.getMessage().getExpiresInSeconds()) { handleSynchronizeSentExpirationUpdate(message); diff --git a/src/org/thoughtcrime/securesms/notifications/AndroidAutoReplyReceiver.java b/src/org/thoughtcrime/securesms/notifications/AndroidAutoReplyReceiver.java index 44b115ebb6..bd8f5eea5c 100644 --- a/src/org/thoughtcrime/securesms/notifications/AndroidAutoReplyReceiver.java +++ b/src/org/thoughtcrime/securesms/notifications/AndroidAutoReplyReceiver.java @@ -71,7 +71,7 @@ public class AndroidAutoReplyReceiver extends BroadcastReceiver { long replyThreadId; int subscriptionId = recipient.getDefaultSubscriptionId().or(-1); - long expiresIn = recipient.getExpireMessages() * 1000; + long expiresIn = recipient.getExpireMessages() * 1000L; if (recipient.isGroupRecipient()) { Log.w("AndroidAutoReplyReceiver", "GroupRecipient, Sending media message"); diff --git a/src/org/thoughtcrime/securesms/notifications/RemoteReplyReceiver.java b/src/org/thoughtcrime/securesms/notifications/RemoteReplyReceiver.java index 5aeb5c51b1..674b08016a 100644 --- a/src/org/thoughtcrime/securesms/notifications/RemoteReplyReceiver.java +++ b/src/org/thoughtcrime/securesms/notifications/RemoteReplyReceiver.java @@ -65,7 +65,7 @@ public class RemoteReplyReceiver extends BroadcastReceiver { Recipient recipient = Recipient.from(context, address, false); int subscriptionId = recipient.getDefaultSubscriptionId().or(-1); - long expiresIn = recipient.getExpireMessages() * 1000; + long expiresIn = recipient.getExpireMessages() * 1000L; if (recipient.isGroupRecipient()) { OutgoingMediaMessage reply = new OutgoingMediaMessage(recipient, responseText.toString(), new LinkedList<>(), System.currentTimeMillis(), subscriptionId, expiresIn, 0); diff --git a/src/org/thoughtcrime/securesms/service/QuickResponseService.java b/src/org/thoughtcrime/securesms/service/QuickResponseService.java index 4ecd05ac56..79029413e3 100644 --- a/src/org/thoughtcrime/securesms/service/QuickResponseService.java +++ b/src/org/thoughtcrime/securesms/service/QuickResponseService.java @@ -50,7 +50,7 @@ public class QuickResponseService extends IntentService { Address address = Address.fromExternal(this, number); Recipient recipient = Recipient.from(this, address, false); int subscriptionId = recipient.getDefaultSubscriptionId().or(-1); - long expiresIn = recipient.getExpireMessages() * 1000; + long expiresIn = recipient.getExpireMessages() * 1000L; if (!TextUtils.isEmpty(content)) { MessageSender.send(this, new OutgoingTextMessage(recipient, content, expiresIn, subscriptionId), -1, false, null);