Add additional protections around recipientIds and threadIds matching.

This commit is contained in:
Greyson Parrelli 2022-12-23 11:26:36 -05:00
parent 4dd8e81db7
commit 3708cc5583
3 changed files with 36 additions and 41 deletions

View file

@ -12,6 +12,7 @@ import org.jsoup.helper.StringUtil
import org.signal.core.util.CursorUtil import org.signal.core.util.CursorUtil
import org.signal.core.util.SqlUtil import org.signal.core.util.SqlUtil
import org.signal.core.util.delete import org.signal.core.util.delete
import org.signal.core.util.exists
import org.signal.core.util.logging.Log import org.signal.core.util.logging.Log
import org.signal.core.util.or import org.signal.core.util.or
import org.signal.core.util.readToList import org.signal.core.util.readToList
@ -1078,7 +1079,11 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
Log.i(TAG, "Using remapped threadId: " + candidateId + " -> " + remapped.get()) Log.i(TAG, "Using remapped threadId: " + candidateId + " -> " + remapped.get())
remapped.get() remapped.get()
} else { } else {
candidateId if (areThreadIdAndRecipientAssociated(candidateId, recipient)) {
candidateId
} else {
throw IllegalArgumentException()
}
} }
} else { } else {
getOrCreateThreadIdFor(recipient, distributionType) getOrCreateThreadIdFor(recipient, distributionType)
@ -1094,6 +1099,13 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
return threadId ?: createThreadForRecipient(recipient.id, recipient.isGroup, distributionType) return threadId ?: createThreadForRecipient(recipient.id, recipient.isGroup, distributionType)
} }
fun areThreadIdAndRecipientAssociated(threadId: Long, recipient: Recipient): Boolean {
return readableDatabase
.exists(TABLE_NAME)
.where("$ID = ? AND $RECIPIENT_ID = ?", threadId, recipient.id)
.run()
}
fun getThreadIdFor(recipientId: RecipientId): Long? { fun getThreadIdFor(recipientId: RecipientId): Long? {
return readableDatabase return readableDatabase
.select(ID) .select(ID)

View file

@ -84,18 +84,25 @@ public class IndividualSendJob extends PushSendJob {
this.messageId = messageId; this.messageId = messageId;
} }
public static Job create(long messageId, @NonNull Recipient recipient, boolean hasMedia) {
if (!recipient.hasServiceId()) {
throw new AssertionError("No ServiceId!");
}
if (recipient.isGroup()) {
throw new AssertionError("This job does not send group messages!");
}
return new IndividualSendJob(messageId, recipient, hasMedia);
}
@WorkerThread @WorkerThread
public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long messageId, @NonNull Recipient recipient) { public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long messageId, @NonNull Recipient recipient) {
try { try {
if (!recipient.hasServiceId()) {
throw new AssertionError("No ServiceId!");
}
OutgoingMessage message = SignalDatabase.messages().getOutgoingMessage(messageId); OutgoingMessage message = SignalDatabase.messages().getOutgoingMessage(messageId);
Set<String> attachmentUploadIds = enqueueCompressingAndUploadAttachmentsChains(jobManager, message); Set<String> attachmentUploadIds = enqueueCompressingAndUploadAttachmentsChains(jobManager, message);
jobManager.add(new IndividualSendJob(messageId, recipient, attachmentUploadIds.size() > 0), attachmentUploadIds, recipient.getId().toQueueKey()); jobManager.add(IndividualSendJob.create(messageId, recipient, attachmentUploadIds.size() > 0), attachmentUploadIds, recipient.getId().toQueueKey());
} catch (NoSuchMessageException | MmsException e) { } catch (NoSuchMessageException | MmsException e) {
Log.w(TAG, "Failed to enqueue message.", e); Log.w(TAG, "Failed to enqueue message.", e);
SignalDatabase.messages().markAsSentFailed(messageId); SignalDatabase.messages().markAsSentFailed(messageId);

View file

@ -260,23 +260,16 @@ public class MessageSender {
Preconditions.checkArgument(message.getAttachments().isEmpty(), "If the media is pre-uploaded, there should be no attachments on the message."); Preconditions.checkArgument(message.getAttachments().isEmpty(), "If the media is pre-uploaded, there should be no attachments on the message.");
try { try {
ThreadTable threadTable = SignalDatabase.threads(); ThreadTable threadTable = SignalDatabase.threads();
MessageTable mmsDatabase = SignalDatabase.messages(); MessageTable mmsDatabase = SignalDatabase.messages();
AttachmentTable attachmentDatabase = SignalDatabase.attachments(); AttachmentTable attachmentDatabase = SignalDatabase.attachments();
long allocatedThreadId; Recipient recipient = message.getRecipient();
long allocatedThreadId = threadTable.getOrCreateValidThreadId(message.getRecipient(), threadId);
if (threadId == -1) { long messageId = mmsDatabase.insertMessageOutbox(applyUniversalExpireTimerIfNecessary(context, recipient, message, allocatedThreadId),
allocatedThreadId = threadTable.getOrCreateThreadIdFor(message.getRecipient(), message.getDistributionType()); allocatedThreadId,
} else { false,
allocatedThreadId = threadId; insertListener);
}
Recipient recipient = message.getRecipient();
long messageId = mmsDatabase.insertMessageOutbox(applyUniversalExpireTimerIfNecessary(context, recipient, message, allocatedThreadId),
allocatedThreadId,
false,
insertListener);
List<AttachmentId> attachmentIds = Stream.of(preUploadResults).map(PreUploadResult::getAttachmentId).toList(); List<AttachmentId> attachmentIds = Stream.of(preUploadResults).map(PreUploadResult::getAttachmentId).toList();
List<String> jobIds = Stream.of(preUploadResults).map(PreUploadResult::getJobIds).flatMap(Stream::of).toList(); List<String> jobIds = Stream.of(preUploadResults).map(PreUploadResult::getJobIds).flatMap(Stream::of).toList();
@ -406,7 +399,7 @@ public class MessageSender {
} else if (recipient.isDistributionList()) { } else if (recipient.isDistributionList()) {
jobManager.add(new PushDistributionListSendJob(messageId, recipient.getId(), true, Collections.emptySet()), messageDependsOnIds, recipient.getId().toQueueKey()); jobManager.add(new PushDistributionListSendJob(messageId, recipient.getId(), true, Collections.emptySet()), messageDependsOnIds, recipient.getId().toQueueKey());
} else { } else {
jobManager.add(new IndividualSendJob(messageId, recipient, true), messageDependsOnIds, recipient.getId().toQueueKey()); jobManager.add(IndividualSendJob.create(messageId, recipient, true), messageDependsOnIds, recipient.getId().toQueueKey());
} }
} }
} }
@ -532,7 +525,7 @@ public class MessageSender {
JobManager jobManager = ApplicationDependencies.getJobManager(); JobManager jobManager = ApplicationDependencies.getJobManager();
if (uploadJobIds.size() > 0) { if (uploadJobIds.size() > 0) {
Job mediaSend = new IndividualSendJob(messageId, recipient, true); Job mediaSend = IndividualSendJob.create(messageId, recipient, true);
jobManager.add(mediaSend, uploadJobIds); jobManager.add(mediaSend, uploadJobIds);
} else { } else {
IndividualSendJob.enqueue(context, jobManager, messageId, recipient); IndividualSendJob.enqueue(context, jobManager, messageId, recipient);
@ -561,28 +554,11 @@ public class MessageSender {
} }
} }
private static void sendSms(Recipient recipient, long messageId) {
JobManager jobManager = ApplicationDependencies.getJobManager();
jobManager.add(new SmsSendJob(messageId, recipient));
}
private static void sendMms(Context context, long messageId) { private static void sendMms(Context context, long messageId) {
JobManager jobManager = ApplicationDependencies.getJobManager(); JobManager jobManager = ApplicationDependencies.getJobManager();
MmsSendJob.enqueue(context, jobManager, messageId); MmsSendJob.enqueue(context, jobManager, messageId);
} }
private static boolean isPushTextSend(Context context, Recipient recipient, boolean keyExchange) {
if (!SignalStore.account().isRegistered()) {
return false;
}
if (keyExchange) {
return false;
}
return isPushDestination(context, recipient);
}
private static boolean isPushMediaSend(Context context, Recipient recipient) { private static boolean isPushMediaSend(Context context, Recipient recipient) {
if (!SignalStore.account().isRegistered()) { if (!SignalStore.account().isRegistered()) {
return false; return false;