From 072185ec36867a6ae8c1fba484c03ae4f7750cf4 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 24 Jul 2026 11:19:49 -0400 Subject: [PATCH] Improve sync message validations. --- .../messages/SyncMessageProcessor.kt | 2 +- .../api/messages/EnvelopeContentValidator.kt | 53 +++- .../messages/EnvelopeContentValidatorTest.kt | 265 ++++++++++++++++++ 3 files changed, 317 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt index 870f96888b..5b9e84822f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/SyncMessageProcessor.kt @@ -2193,7 +2193,7 @@ object SyncMessageProcessor { } private fun AddressableMessage.toSyncMessageId(envelopeTimestamp: Long): MessageTable.SyncMessageId? { - return if (this.sentTimestamp != null && Utils.anyNotNull(this.authorServiceId, this.authorServiceIdBinary) || this.authorE164 != null) { + return if (this.sentTimestamp != null && (Utils.anyNotNull(this.authorServiceId, this.authorServiceIdBinary) || this.authorE164 != null)) { val serviceId = ServiceId.parseOrNull(this.authorServiceId, this.authorServiceIdBinary) val id = if (serviceId != null) { SignalDatabase.recipients.getOrInsertFromServiceId(serviceId) diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidator.kt b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidator.kt index 688a769269..94fef3b7ea 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidator.kt +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidator.kt @@ -216,6 +216,10 @@ object EnvelopeContentValidator { } if (syncMessage.sent != null) { + if (syncMessage.sent.timestamp == null) { + return Result.Invalid("[SyncMessage] Missing timestamp on SyncMessage.sent!") + } + val validAddress = ServiceId.parseOrNull(syncMessage.sent.destinationServiceId, syncMessage.sent.destinationServiceIdBinary) != null val hasDataGroup = syncMessage.sent.message?.groupV2 != null val hasStoryGroup = syncMessage.sent.storyMessage?.group != null @@ -244,6 +248,22 @@ object EnvelopeContentValidator { } } + for (recipient in syncMessage.sent.storyMessageRecipients) { + if (Util.anyNotNull(recipient.destinationServiceId, recipient.destinationServiceIdBinary)) { + if (ServiceId.parseOrNull(recipient.destinationServiceId, recipient.destinationServiceIdBinary) == null) { + return Result.Invalid("[SyncMessage] Invalid destination ServiceId in SyncMessage.sent.storyMessageRecipients!") + } + + if (recipient.isAllowedToReply == null) { + return Result.Invalid("[SyncMessage] Missing isAllowedToReply in SyncMessage.sent.storyMessageRecipients!") + } + } + } + + if (hasStoryManifest && syncMessage.sent.storyMessage == null && syncMessage.sent.isRecipientUpdate != true) { + return Result.Invalid("[SyncMessage] SyncMessage.sent had story recipients but no story message and was not a recipient update!") + } + return if (syncMessage.sent.message != null) { validateDataMessage(envelope, syncMessage.sent.message) } else if (syncMessage.sent.storyMessage != null) { @@ -261,6 +281,10 @@ object EnvelopeContentValidator { return Result.Invalid("[SyncMessage] Invalid ACI in SyncMessage.readList!") } + if (syncMessage.read.any { it.timestamp == null }) { + return Result.Invalid("[SyncMessage] Missing timestamp in SyncMessage.readList!") + } + if (syncMessage.viewed.any { ACI.parseOrNull(it.senderAci, it.senderAciBinary).isNullOrInvalidServiceId() }) { return Result.Invalid("[SyncMessage] Invalid ACI in SyncMessage.viewList!") } @@ -285,8 +309,33 @@ object EnvelopeContentValidator { return Result.Invalid("[SyncMessage] Invalid ACI in SyncMessage.messageRequestResponse!") } - if (syncMessage.outgoingPayment != null && syncMessage.outgoingPayment.recipientServiceId.isNullOrInvalidServiceId()) { - return Result.Invalid("[SyncMessage] Invalid ServiceId in SyncMessage.outgoingPayment!") + if (syncMessage.outgoingPayment != null) { + if (syncMessage.outgoingPayment.recipientServiceId.isNullOrInvalidServiceId()) { + return Result.Invalid("[SyncMessage] Invalid ServiceId in SyncMessage.outgoingPayment!") + } + + val mobileCoin = syncMessage.outgoingPayment.mobileCoin + if (mobileCoin != null && (mobileCoin.recipientAddress == null || mobileCoin.amountPicoMob == null || mobileCoin.feePicoMob == null || mobileCoin.receipt == null || mobileCoin.ledgerBlockIndex == null)) { + return Result.Invalid("[SyncMessage] Missing required MobileCoin field in SyncMessage.outgoingPayment!") + } + } + + if (syncMessage.callEvent != null && syncMessage.callEvent.callId != null) { + val callEvent = syncMessage.callEvent + val isOneToOne = callEvent.type == SyncMessage.CallEvent.Type.AUDIO_CALL || callEvent.type == SyncMessage.CallEvent.Type.VIDEO_CALL + + if (isOneToOne && callEvent.conversationId != null && ACI.parseOrNull(callEvent.conversationId) == null) { + return Result.Invalid("[SyncMessage] Invalid ACI conversationId in SyncMessage.callEvent!") + } + + if (callEvent.type == SyncMessage.CallEvent.Type.AD_HOC_CALL && + callEvent.event == SyncMessage.CallEvent.Event.OBSERVED && + callEvent.direction == SyncMessage.CallEvent.Direction.INCOMING && + callEvent.conversationId != null && + callEvent.timestamp == null + ) { + return Result.Invalid("[SyncMessage] Missing timestamp on observed ad-hoc call event in SyncMessage.callEvent!") + } } return Result.Valid diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidatorTest.kt b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidatorTest.kt index 9ba6c54369..545241bf9a 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidatorTest.kt +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/EnvelopeContentValidatorTest.kt @@ -837,4 +837,269 @@ class EnvelopeContentValidatorTest { val result = EnvelopeContentValidator.validate(Envelope(clientTimestamp = 1234), content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) assert(result is EnvelopeContentValidator.Result.Valid) } + + @Test + fun `validate - ensure sync sent without a timestamp is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString(), clientTimestamp = 1234) + val content = Content( + syncMessage = SyncMessage( + sent = SyncMessage.Sent( + destinationServiceId = OTHER_ACI.toString(), + message = DataMessage(timestamp = 1234) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync sent with a timestamp and valid destination is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString(), clientTimestamp = 1234) + val content = Content( + syncMessage = SyncMessage( + sent = SyncMessage.Sent( + timestamp = 1234, + destinationServiceId = OTHER_ACI.toString(), + message = DataMessage(timestamp = 1234) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure sync sent with story recipients but no story message and not a recipient update is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString(), clientTimestamp = 1234) + val content = Content( + syncMessage = SyncMessage( + sent = SyncMessage.Sent( + timestamp = 1234, + storyMessageRecipients = listOf( + SyncMessage.Sent.StoryMessageRecipient( + destinationServiceId = OTHER_ACI.toString(), + isAllowedToReply = true + ) + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync sent story recipient update without a story message is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString(), clientTimestamp = 1234) + val content = Content( + syncMessage = SyncMessage( + sent = SyncMessage.Sent( + timestamp = 1234, + isRecipientUpdate = true, + storyMessageRecipients = listOf( + SyncMessage.Sent.StoryMessageRecipient( + destinationServiceId = OTHER_ACI.toString(), + isAllowedToReply = true + ) + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure sync sent story recipient with an invalid destination is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString(), clientTimestamp = 1234) + val content = Content( + syncMessage = SyncMessage( + sent = SyncMessage.Sent( + timestamp = 1234, + storyMessage = StoryMessage(), + storyMessageRecipients = listOf( + SyncMessage.Sent.StoryMessageRecipient( + destinationServiceId = "not-a-uuid", + isAllowedToReply = true + ) + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync sent story recipient without isAllowedToReply is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString(), clientTimestamp = 1234) + val content = Content( + syncMessage = SyncMessage( + sent = SyncMessage.Sent( + timestamp = 1234, + storyMessage = StoryMessage(), + storyMessageRecipients = listOf( + SyncMessage.Sent.StoryMessageRecipient( + destinationServiceId = OTHER_ACI.toString() + ) + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync read without a timestamp is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + read = listOf( + SyncMessage.Read(senderAci = OTHER_ACI.toString()) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync read with a valid aci and timestamp is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + read = listOf( + SyncMessage.Read(senderAci = OTHER_ACI.toString(), timestamp = 1000) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure sync outgoing payment with mobileCoin missing required fields is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + outgoingPayment = SyncMessage.OutgoingPayment( + recipientServiceId = OTHER_ACI.toString(), + mobileCoin = SyncMessage.OutgoingPayment.MobileCoin( + recipientAddress = "address".toByteArray().toByteString(), + amountPicoMob = 1, + feePicoMob = 1, + receipt = "receipt".toByteArray().toByteString() + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync outgoing payment with all mobileCoin fields is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + outgoingPayment = SyncMessage.OutgoingPayment( + recipientServiceId = OTHER_ACI.toString(), + mobileCoin = SyncMessage.OutgoingPayment.MobileCoin( + recipientAddress = "address".toByteArray().toByteString(), + amountPicoMob = 1, + feePicoMob = 1, + receipt = "receipt".toByteArray().toByteString(), + ledgerBlockIndex = 1 + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure one-to-one call event with a malformed conversationId is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + callEvent = SyncMessage.CallEvent( + callId = 1, + type = SyncMessage.CallEvent.Type.AUDIO_CALL, + conversationId = "bad".toByteArray().toByteString() + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure one-to-one call event with a valid aci conversationId is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + callEvent = SyncMessage.CallEvent( + callId = 1, + type = SyncMessage.CallEvent.Type.AUDIO_CALL, + conversationId = OTHER_ACI.toByteString() + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure observed ad-hoc call event without a timestamp is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + callEvent = SyncMessage.CallEvent( + callId = 1, + type = SyncMessage.CallEvent.Type.AD_HOC_CALL, + event = SyncMessage.CallEvent.Event.OBSERVED, + direction = SyncMessage.CallEvent.Direction.INCOMING, + conversationId = "room".toByteArray().toByteString() + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure observed ad-hoc call event with a timestamp is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + callEvent = SyncMessage.CallEvent( + callId = 1, + type = SyncMessage.CallEvent.Type.AD_HOC_CALL, + event = SyncMessage.CallEvent.Event.OBSERVED, + direction = SyncMessage.CallEvent.Direction.INCOMING, + conversationId = "room".toByteArray().toByteString(), + timestamp = 1000 + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } }