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
This commit is contained in:
Sam Lanning 2018-01-12 11:39:10 +00:00 committed by Moxie Marlinspike
parent 10c1ee70e8
commit 69f180a5ec
5 changed files with 13 additions and 13 deletions

View File

@ -589,7 +589,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
@Override @Override
protected Void doInBackground(Void... params) { protected Void doInBackground(Void... params) {
DatabaseFactory.getRecipientDatabase(ConversationActivity.this).setExpireMessages(recipient, expirationTime); 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); MessageSender.send(ConversationActivity.this, outgoingMessage, threadId, false, null);
return null; return null;
@ -1627,7 +1627,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
boolean forceSms = sendButton.isManualSelection() && sendButton.getSelectedTransport().isSms(); boolean forceSms = sendButton.isManualSelection() && sendButton.getSelectedTransport().isSms();
int subscriptionId = sendButton.getSelectedTransport().getSimSubscriptionId().or(-1); int subscriptionId = sendButton.getSelectedTransport().getSimSubscriptionId().or(-1);
long expiresIn = recipient.getExpireMessages() * 1000; long expiresIn = recipient.getExpireMessages() * 1000L;
boolean initiating = threadId == -1; boolean initiating = threadId == -1;
Log.w(TAG, "isManual Selection: " + sendButton.isManualSelection()); Log.w(TAG, "isManual Selection: " + sendButton.isManualSelection());
@ -1842,7 +1842,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
public void onSuccess(final @NonNull Pair<Uri, Long> result) { public void onSuccess(final @NonNull Pair<Uri, Long> result) {
boolean forceSms = sendButton.isManualSelection() && sendButton.getSelectedTransport().isSms(); boolean forceSms = sendButton.isManualSelection() && sendButton.getSelectedTransport().isSms();
int subscriptionId = sendButton.getSelectedTransport().getSimSubscriptionId().or(-1); int subscriptionId = sendButton.getSelectedTransport().getSimSubscriptionId().or(-1);
long expiresIn = recipient.getExpireMessages() * 1000; long expiresIn = recipient.getExpireMessages() * 1000L;
boolean initiating = threadId == -1; boolean initiating = threadId == -1;
AudioSlide audioSlide = new AudioSlide(ConversationActivity.this, result.first, result.second, MediaUtil.AUDIO_AAC, true); AudioSlide audioSlide = new AudioSlide(ConversationActivity.this, result.first, result.second, MediaUtil.AUDIO_AAC, true);
SlideDeck slideDeck = new SlideDeck(); SlideDeck slideDeck = new SlideDeck();

View File

@ -395,7 +395,7 @@ public class PushDecryptJob extends ContextJob {
Recipient recipient = getMessageDestination(envelope, message); Recipient recipient = getMessageDestination(envelope, message);
IncomingMediaMessage mediaMessage = new IncomingMediaMessage(Address.fromExternal(context, envelope.getSource()), IncomingMediaMessage mediaMessage = new IncomingMediaMessage(Address.fromExternal(context, envelope.getSource()),
message.getTimestamp(), -1, message.getTimestamp(), -1,
message.getExpiresInSeconds() * 1000, true, message.getExpiresInSeconds() * 1000L, true,
Optional.fromNullable(envelope.getRelay()), Optional.fromNullable(envelope.getRelay()),
Optional.absent(), message.getGroupInfo(), Optional.absent(), message.getGroupInfo(),
Optional.absent()); Optional.absent());
@ -519,7 +519,7 @@ public class PushDecryptJob extends ContextJob {
Recipient recipient = getMessageDestination(envelope, message); Recipient recipient = getMessageDestination(envelope, message);
IncomingMediaMessage mediaMessage = new IncomingMediaMessage(Address.fromExternal(context, envelope.getSource()), IncomingMediaMessage mediaMessage = new IncomingMediaMessage(Address.fromExternal(context, envelope.getSource()),
message.getTimestamp(), -1, message.getTimestamp(), -1,
message.getExpiresInSeconds() * 1000, false, message.getExpiresInSeconds() * 1000L, false,
Optional.fromNullable(envelope.getRelay()), Optional.fromNullable(envelope.getRelay()),
message.getBody(), message.getBody(),
message.getGroupInfo(), message.getGroupInfo(),
@ -556,7 +556,7 @@ public class PushDecryptJob extends ContextJob {
OutgoingExpirationUpdateMessage expirationUpdateMessage = new OutgoingExpirationUpdateMessage(recipient, OutgoingExpirationUpdateMessage expirationUpdateMessage = new OutgoingExpirationUpdateMessage(recipient,
message.getTimestamp(), message.getTimestamp(),
message.getMessage().getExpiresInSeconds() * 1000); message.getMessage().getExpiresInSeconds() * 1000L);
long threadId = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipient); long threadId = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipient);
long messageId = database.insertMessageOutbox(expirationUpdateMessage, threadId, false, null); 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(), OutgoingMediaMessage mediaMessage = new OutgoingMediaMessage(recipients, message.getMessage().getBody().orNull(),
PointerAttachment.forPointers(message.getMessage().getAttachments()), PointerAttachment.forPointers(message.getMessage().getAttachments()),
message.getTimestamp(), -1, message.getTimestamp(), -1,
message.getMessage().getExpiresInSeconds() * 1000, message.getMessage().getExpiresInSeconds() * 1000L,
ThreadDatabase.DistributionTypes.DEFAULT); ThreadDatabase.DistributionTypes.DEFAULT);
mediaMessage = new OutgoingSecureMediaMessage(mediaMessage); mediaMessage = new OutgoingSecureMediaMessage(mediaMessage);
@ -602,7 +602,7 @@ public class PushDecryptJob extends ContextJob {
.getExpiringMessageManager() .getExpiringMessageManager()
.scheduleDeletion(messageId, true, .scheduleDeletion(messageId, true,
message.getExpirationStartTimestamp(), message.getExpirationStartTimestamp(),
message.getMessage().getExpiresInSeconds() * 1000); message.getMessage().getExpiresInSeconds() * 1000L);
} }
return threadId; return threadId;
@ -630,7 +630,7 @@ public class PushDecryptJob extends ContextJob {
envelope.getSourceDevice(), envelope.getSourceDevice(),
message.getTimestamp(), body, message.getTimestamp(), body,
message.getGroupInfo(), message.getGroupInfo(),
message.getExpiresInSeconds() * 1000); message.getExpiresInSeconds() * 1000L);
textMessage = new IncomingEncryptedMessage(textMessage, body); textMessage = new IncomingEncryptedMessage(textMessage, body);
Optional<InsertResult> insertResult = database.insertMessageInbox(textMessage); Optional<InsertResult> insertResult = database.insertMessageInbox(textMessage);
@ -652,7 +652,7 @@ public class PushDecryptJob extends ContextJob {
Recipient recipient = getSyncMessageDestination(message); Recipient recipient = getSyncMessageDestination(message);
String body = message.getMessage().getBody().or(""); String body = message.getMessage().getBody().or("");
long expiresInMillis = message.getMessage().getExpiresInSeconds() * 1000; long expiresInMillis = message.getMessage().getExpiresInSeconds() * 1000L;
if (recipient.getExpireMessages() != message.getMessage().getExpiresInSeconds()) { if (recipient.getExpireMessages() != message.getMessage().getExpiresInSeconds()) {
handleSynchronizeSentExpirationUpdate(message); handleSynchronizeSentExpirationUpdate(message);

View File

@ -71,7 +71,7 @@ public class AndroidAutoReplyReceiver extends BroadcastReceiver {
long replyThreadId; long replyThreadId;
int subscriptionId = recipient.getDefaultSubscriptionId().or(-1); int subscriptionId = recipient.getDefaultSubscriptionId().or(-1);
long expiresIn = recipient.getExpireMessages() * 1000; long expiresIn = recipient.getExpireMessages() * 1000L;
if (recipient.isGroupRecipient()) { if (recipient.isGroupRecipient()) {
Log.w("AndroidAutoReplyReceiver", "GroupRecipient, Sending media message"); Log.w("AndroidAutoReplyReceiver", "GroupRecipient, Sending media message");

View File

@ -65,7 +65,7 @@ public class RemoteReplyReceiver extends BroadcastReceiver {
Recipient recipient = Recipient.from(context, address, false); Recipient recipient = Recipient.from(context, address, false);
int subscriptionId = recipient.getDefaultSubscriptionId().or(-1); int subscriptionId = recipient.getDefaultSubscriptionId().or(-1);
long expiresIn = recipient.getExpireMessages() * 1000; long expiresIn = recipient.getExpireMessages() * 1000L;
if (recipient.isGroupRecipient()) { if (recipient.isGroupRecipient()) {
OutgoingMediaMessage reply = new OutgoingMediaMessage(recipient, responseText.toString(), new LinkedList<>(), System.currentTimeMillis(), subscriptionId, expiresIn, 0); OutgoingMediaMessage reply = new OutgoingMediaMessage(recipient, responseText.toString(), new LinkedList<>(), System.currentTimeMillis(), subscriptionId, expiresIn, 0);

View File

@ -50,7 +50,7 @@ public class QuickResponseService extends IntentService {
Address address = Address.fromExternal(this, number); Address address = Address.fromExternal(this, number);
Recipient recipient = Recipient.from(this, address, false); Recipient recipient = Recipient.from(this, address, false);
int subscriptionId = recipient.getDefaultSubscriptionId().or(-1); int subscriptionId = recipient.getDefaultSubscriptionId().or(-1);
long expiresIn = recipient.getExpireMessages() * 1000; long expiresIn = recipient.getExpireMessages() * 1000L;
if (!TextUtils.isEmpty(content)) { if (!TextUtils.isEmpty(content)) {
MessageSender.send(this, new OutgoingTextMessage(recipient, content, expiresIn, subscriptionId), -1, false, null); MessageSender.send(this, new OutgoingTextMessage(recipient, content, expiresIn, subscriptionId), -1, false, null);