From 88b21b611318f678633f83e57d9e9ddfddec7b32 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 8 Apr 2026 15:49:22 -0400 Subject: [PATCH] Improve validator testing. --- .../messages/DataMessageProcessor.kt | 7 +- .../api/messages/EnvelopeContentValidator.kt | 9 +- .../messages/EnvelopeContentValidatorTest.kt | 125 ++++++++++++++++++ 3 files changed, 136 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt index 678a0bb687..259af11fe3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt @@ -1252,7 +1252,7 @@ object DataMessageProcessor { SignalDatabase.polls.insertVotes( pollId = pollId, - pollOptionIds = pollVote.optionIndexes.map { index -> allOptionIds[index] }, + pollOptionIds = pollVote.optionIndexes.distinct().map { index -> allOptionIds[index] }, voterId = senderRecipient.id.toLong(), voteCount = pollVote.voteCount?.toLong() ?: 0, messageId = messageId @@ -1586,15 +1586,16 @@ object DataMessageProcessor { } warn(timestamp, "Didn't find matching message record...") + val cappedQuoteRanges = quote.bodyRanges.take(BODY_RANGE_PROCESSING_LIMIT) return QuoteModel( id = quote.id!!, author = authorId, text = quote.text ?: "", isOriginalMissing = true, attachment = quote.attachments.firstNotNullOfOrNull { PointerAttachment.forPointer(it).orNull() }, - mentions = getMentions(quote.bodyRanges), + mentions = getMentions(cappedQuoteRanges), type = QuoteModel.Type.fromProto(quote.type), - bodyRanges = quote.bodyRanges.filter { Util.allAreNull(it.mentionAci, it.mentionAciBinary) }.toBodyRangeList() + bodyRanges = cappedQuoteRanges.filter { Util.allAreNull(it.mentionAci, it.mentionAciBinary) }.toBodyRangeList() ) } 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 95481e0043..2c08e6b8aa 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 @@ -33,6 +33,7 @@ object EnvelopeContentValidator { private const val MAX_POLL_QUESTION_CHARACTER_LENGTH = 200 private const val MAX_POLL_CHARACTER_LENGTH = 100 private const val MIN_POLL_OPTIONS = 2 + private const val MAX_POLL_OPTIONS = 10 fun validate(envelope: Envelope, content: Content, localAci: ACI, ciphertextMessageType: Int): Result { if (envelope.type == Envelope.Type.PLAINTEXT_CONTENT || ciphertextMessageType == CiphertextMessage.PLAINTEXT_CONTENT_TYPE) { @@ -91,6 +92,10 @@ object EnvelopeContentValidator { return Result.Invalid("[DataMessage] Invalid ACI on quote!") } + if (dataMessage.quote != null && dataMessage.quote.bodyRanges.any { Util.anyNotNull(it.mentionAci, it.mentionAciBinary) && ACI.parseOrNull(it.mentionAci, it.mentionAciBinary).isNullOrInvalidServiceId() }) { + return Result.Invalid("[DataMessage] Invalid ACI on quote body range!") + } + if (dataMessage.contact.any { it.avatar != null && it.avatar.avatar.isPresentAndInvalid() }) { return Result.Invalid("[DataMessage] Invalid AttachmentPointer on DataMessage.contactList.avatar!") } @@ -153,7 +158,7 @@ object EnvelopeContentValidator { return Result.Invalid("[DataMessage] Invalid poll terminate!") } - if (dataMessage.pollVote != null && (dataMessage.pollVote.targetAuthorAciBinary.isNullOrInvalidAci() || dataMessage.pollVote.targetSentTimestamp == null || dataMessage.pollVote.voteCount == null)) { + if (dataMessage.pollVote != null && (dataMessage.pollVote.targetAuthorAciBinary.isNullOrInvalidAci() || dataMessage.pollVote.targetSentTimestamp == null || dataMessage.pollVote.voteCount == null || dataMessage.pollVote.optionIndexes.size > MAX_POLL_OPTIONS)) { return Result.Invalid("[DataMessage] Invalid poll vote!") } @@ -250,7 +255,7 @@ object EnvelopeContentValidator { return Result.Invalid("[SyncMessage] Missing packId in stickerPackOperationList!") } - if (syncMessage.blocked != null && syncMessage.blocked.acis.any { it.isNullOrInvalidAci() } && syncMessage.blocked.acisBinary.any { it.isNullOrInvalidAci() }) { + if (syncMessage.blocked != null && (syncMessage.blocked.acis.any { it.isNullOrInvalidAci() } || syncMessage.blocked.acisBinary.any { it.isNullOrInvalidAci() })) { return Result.Invalid("[SyncMessage] Invalid ACI in SyncMessage.blocked!") } 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 6c59e8a1f6..cd97c19ba8 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 @@ -10,14 +10,17 @@ import org.junit.Test import org.signal.core.models.ServiceId import org.signal.libsignal.protocol.message.CiphertextMessage import org.signal.libsignal.protocol.message.DecryptionErrorMessage +import org.whispersystems.signalservice.internal.push.BodyRange import org.whispersystems.signalservice.internal.push.Content import org.whispersystems.signalservice.internal.push.DataMessage import org.whispersystems.signalservice.internal.push.Envelope +import org.whispersystems.signalservice.internal.push.SyncMessage class EnvelopeContentValidatorTest { companion object { private val SELF_ACI = ServiceId.ACI.parseOrThrow("0a5ebe7e-9de7-41a5-a25f-6ace4f8e11d1") + private val OTHER_ACI = ServiceId.ACI.parseOrThrow("11111111-9de7-41a5-a25f-6ace4f8e11d1") } @Test @@ -261,4 +264,126 @@ class EnvelopeContentValidatorTest { ) return decryptionErrorMessage.serialize().toByteString() } + + @Test + fun `validate - ensure poll votes with too many option indexes are marked invalid`() { + val content = Content( + dataMessage = DataMessage( + timestamp = 1234, + pollVote = DataMessage.PollVote( + targetAuthorAciBinary = OTHER_ACI.toByteString(), + targetSentTimestamp = 1000, + voteCount = 1, + optionIndexes = List(11) { 0 } + ) + ) + ) + + val result = EnvelopeContentValidator.validate(Envelope(clientTimestamp = 1234), content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure poll votes with max option indexes are marked valid`() { + val content = Content( + dataMessage = DataMessage( + timestamp = 1234, + pollVote = DataMessage.PollVote( + targetAuthorAciBinary = OTHER_ACI.toByteString(), + targetSentTimestamp = 1000, + voteCount = 1, + optionIndexes = List(10) { it } + ) + ) + ) + + val result = EnvelopeContentValidator.validate(Envelope(clientTimestamp = 1234), content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure quote body range mentions with invalid aci are marked invalid`() { + val content = Content( + dataMessage = DataMessage( + timestamp = 1234, + quote = DataMessage.Quote( + id = 1000, + authorAci = OTHER_ACI.toString(), + bodyRanges = listOf( + BodyRange(start = 0, length = 1, mentionAci = "not-a-uuid") + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(Envelope(clientTimestamp = 1234), content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure quote body range mentions with valid aci are marked valid`() { + val content = Content( + dataMessage = DataMessage( + timestamp = 1234, + quote = DataMessage.Quote( + id = 1000, + authorAci = OTHER_ACI.toString(), + bodyRanges = listOf( + BodyRange(start = 0, length = 1, mentionAci = OTHER_ACI.toString()) + ) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(Envelope(clientTimestamp = 1234), content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } + + @Test + fun `validate - ensure sync blocked with invalid string aci and empty binary list is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + blocked = SyncMessage.Blocked( + acis = listOf("not-a-uuid"), + acisBinary = emptyList() + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync blocked with invalid binary aci and empty string list is marked invalid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + blocked = SyncMessage.Blocked( + acis = emptyList(), + acisBinary = listOf("bad".toByteArray().toByteString()) + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Invalid) + } + + @Test + fun `validate - ensure sync blocked with valid acis is marked valid`() { + val envelope = Envelope(sourceServiceId = SELF_ACI.toString()) + val content = Content( + syncMessage = SyncMessage( + blocked = SyncMessage.Blocked( + acis = listOf(OTHER_ACI.toString()), + acisBinary = emptyList() + ) + ) + ) + + val result = EnvelopeContentValidator.validate(envelope, content, SELF_ACI, CiphertextMessage.WHISPER_TYPE) + assert(result is EnvelopeContentValidator.Result.Valid) + } }