Protect against unknown GV2 UUIDs.

This commit is contained in:
Alan Evans 2020-05-19 15:41:48 -03:00 committed by Greyson Parrelli
parent 981676c7f8
commit ec8d5defd4
5 changed files with 71 additions and 45 deletions

View File

@ -30,7 +30,6 @@ import org.thoughtcrime.securesms.recipients.RecipientId;
import org.whispersystems.libsignal.util.guava.Optional;
import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil;
import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer;
import org.whispersystems.signalservice.api.push.SignalServiceAddress;
import org.whispersystems.signalservice.api.util.UuidUtil;
import java.io.Closeable;
@ -41,6 +40,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.UUID;
public final class GroupDatabase extends Database {
@ -252,7 +252,7 @@ public final class GroupDatabase extends Database {
@WorkerThread
public @NonNull List<Recipient> getGroupMembers(@NonNull GroupId groupId, @NonNull MemberSet memberSet) {
if (groupId.isV2()) {
return getGroup(groupId).transform(g -> g.requireV2GroupProperties().getMembers(context, memberSet))
return getGroup(groupId).transform(g -> g.requireV2GroupProperties().getMemberRecipients(memberSet))
.or(Collections.emptyList());
} else {
List<RecipientId> currentMembers = getCurrentMembers(groupId);
@ -338,7 +338,7 @@ public final class GroupDatabase extends Database {
contentValues.put(V2_MASTER_KEY, groupMasterKey.serialize());
contentValues.put(V2_REVISION, groupState.getVersion());
contentValues.put(V2_DECRYPTED_GROUP, groupState.toByteArray());
contentValues.put(MEMBERS, serializeV2GroupMembers(context, groupState));
contentValues.put(MEMBERS, serializeV2GroupMembers(groupState));
} else {
if (groupId.isV2()) {
throw new AssertionError("V2 group id but no master key");
@ -395,7 +395,7 @@ public final class GroupDatabase extends Database {
contentValues.put(TITLE, title);
contentValues.put(V2_REVISION, decryptedGroup.getVersion());
contentValues.put(V2_DECRYPTED_GROUP, decryptedGroup.toByteArray());
contentValues.put(MEMBERS, serializeV2GroupMembers(context, decryptedGroup));
contentValues.put(MEMBERS, serializeV2GroupMembers(decryptedGroup));
databaseHelper.getWritableDatabase().update(TABLE_NAME, contentValues,
GROUP_ID + " = ?",
@ -517,13 +517,16 @@ public final class GroupDatabase extends Database {
return getGroup(groupId).transform(g -> g.isPendingMember(recipient)).or(false);
}
private static String serializeV2GroupMembers(@NonNull Context context, @NonNull DecryptedGroup decryptedGroup) {
private static String serializeV2GroupMembers(@NonNull DecryptedGroup decryptedGroup) {
List<RecipientId> groupMembers = new ArrayList<>(decryptedGroup.getMembersCount());
for (DecryptedMember member : decryptedGroup.getMembersList()) {
Recipient recipient = Recipient.externalPush(context, new SignalServiceAddress(UuidUtil.fromByteString(member.getUuid()), null));
groupMembers.add(recipient.getId());
UUID uuid = UuidUtil.fromByteString(member.getUuid());
if (UuidUtil.UNKNOWN_UUID.equals(uuid)) {
Log.w(TAG, "Seen unknown UUID in members list");
} else {
groupMembers.add(RecipientId.from(uuid, null));
}
}
Collections.sort(groupMembers);
@ -779,25 +782,41 @@ public final class GroupDatabase extends Database {
.or(false);
}
public List<Recipient> getMembers(@NonNull Context context, @NonNull MemberSet memberSet) {
public List<Recipient> getMemberRecipients(@NonNull MemberSet memberSet) {
return Stream.of(getMemberRecipientIds(memberSet))
.map(Recipient::resolved)
.toList();
}
public List<RecipientId> getMemberRecipientIds(@NonNull MemberSet memberSet) {
boolean includeSelf = memberSet.includeSelf;
DecryptedGroup groupV2 = getDecryptedGroup();
UUID selfUuid = Recipient.self().getUuid().get();
List<Recipient> recipients = new ArrayList<>(groupV2.getMembersCount() + groupV2.getPendingMembersCount());
List<RecipientId> recipients = new ArrayList<>(groupV2.getMembersCount() + groupV2.getPendingMembersCount());
int unknownMembers = 0;
int unknownPending = 0;
for (UUID uuid : DecryptedGroupUtil.toUuidList(groupV2.getMembersList())) {
if (includeSelf || !selfUuid.equals(uuid)) {
recipients.add(Recipient.externalPush(context, uuid, null));
if (UuidUtil.UNKNOWN_UUID.equals(uuid)) {
unknownMembers++;
} else if (includeSelf || !selfUuid.equals(uuid)) {
recipients.add(RecipientId.from(uuid, null));
}
}
if (memberSet.includePending) {
for (UUID uuid : DecryptedGroupUtil.pendingToUuidList(groupV2.getPendingMembersList())) {
if (includeSelf || !selfUuid.equals(uuid)) {
recipients.add(Recipient.externalPush(context, uuid, null));
if (UuidUtil.UNKNOWN_UUID.equals(uuid)) {
unknownPending++;
} else if (includeSelf || !selfUuid.equals(uuid)) {
recipients.add(RecipientId.from(uuid, null));
}
}
}
if ((unknownMembers + unknownPending) > 0) {
Log.w(TAG, String.format(Locale.US, "Group contains %d + %d unknown pending and full members", unknownPending, unknownMembers));
}
return recipients;
}
}

View File

@ -1083,9 +1083,7 @@ public class MmsDatabase extends MessagingDatabase {
if (message.isGroup()) {
OutgoingGroupUpdateMessage outgoingGroupUpdateMessage = (OutgoingGroupUpdateMessage) message;
if (outgoingGroupUpdateMessage.isV2Group()) {
MessageGroupContext.GroupV2Properties groupV2Properties = outgoingGroupUpdateMessage.requireGroupV2Properties();
type |= Types.GROUP_V2_BIT;
if (groupV2Properties.isUpdate()) type |= Types.GROUP_UPDATE_BIT;
type |= Types.GROUP_V2_BIT | Types.GROUP_UPDATE_BIT;
} else {
MessageGroupContext.GroupV1Properties properties = outgoingGroupUpdateMessage.requireGroupV1Properties();
if (properties.isUpdate()) type |= Types.GROUP_UPDATE_BIT;
@ -1130,19 +1128,14 @@ public class MmsDatabase extends MessagingDatabase {
OutgoingGroupUpdateMessage outgoingGroupUpdateMessage = (message instanceof OutgoingGroupUpdateMessage) ? (OutgoingGroupUpdateMessage) message : null;
GroupReceiptDatabase receiptDatabase = DatabaseFactory.getGroupReceiptDatabase(context);
RecipientDatabase recipientDatabase = DatabaseFactory.getRecipientDatabase(context);
Set<RecipientId> members = new HashSet<>();
if (outgoingGroupUpdateMessage != null && outgoingGroupUpdateMessage.isV2Group()) {
MessageGroupContext.GroupV2Properties groupV2Properties = outgoingGroupUpdateMessage.requireGroupV2Properties();
members.addAll(Stream.of(groupV2Properties.getActiveMembers()).map(recipientDatabase::getOrInsertFromUuid).toList());
if (groupV2Properties.isUpdate()) {
members.addAll(Stream.concat(Stream.of(groupV2Properties.getPendingMembers()),
Stream.of(groupV2Properties.getRemovedMembers()))
members.addAll(Stream.of(groupV2Properties.getAllActivePendingAndRemovedMembers())
.distinct()
.map(recipientDatabase::getOrInsertFromUuid)
.map(uuid -> RecipientId.from(uuid, null))
.toList());
}
members.remove(Recipient.self().getId());
} else {
members.addAll(Stream.of(DatabaseFactory.getGroupDatabase(context).getGroupMembers(message.getRecipient().requireGroupId(), GroupDatabase.MemberSet.FULL_MEMBERS_EXCLUDING_SELF)).map(Recipient::getId).toList());

View File

@ -37,6 +37,12 @@ public final class ProfileKeySet {
{
for (DecryptedMember member : group.getMembersList()) {
UUID memberUuid = UuidUtil.fromByteString(member.getUuid());
if (UuidUtil.UNKNOWN_UUID.equals(memberUuid)) {
Log.w(TAG, "Seen unknown member UUID");
continue;
}
ProfileKey profileKey;
try {
profileKey = new ProfileKey(member.getProfileKey().toByteArray());

View File

@ -164,21 +164,14 @@ public final class MessageGroupContext {
return groupMasterKey;
}
public @NonNull List<UUID> getActiveMembers() {
return DecryptedGroupUtil.membersToUuidList(decryptedGroupV2Context.getGroupState().getMembersList());
}
public @NonNull List<UUID> getAllActivePendingAndRemovedMembers() {
LinkedList<UUID> memberUuids = new LinkedList<>();
public @NonNull List<UUID> getPendingMembers() {
return DecryptedGroupUtil.pendingToUuidList(decryptedGroupV2Context.getGroupState().getPendingMembersList());
}
memberUuids.addAll(DecryptedGroupUtil.membersToUuidList(decryptedGroupV2Context.getGroupState().getMembersList()));
memberUuids.addAll(DecryptedGroupUtil.pendingToUuidList(decryptedGroupV2Context.getGroupState().getPendingMembersList()));
memberUuids.addAll(DecryptedGroupUtil.removedMembersUuidList(decryptedGroupV2Context.getChange()));
public @NonNull List<UUID> getRemovedMembers() {
return DecryptedGroupUtil.removedMembersUuidList(decryptedGroupV2Context.getChange());
}
public boolean isUpdate() {
// The group context is only stored on update messages.
return true;
return UuidUtil.filterKnown(memberUuids);
}
@Override

View File

@ -81,4 +81,19 @@ public final class UuidUtil {
return result;
}
/**
* Keep only UUIDs that are not the {@link #UNKNOWN_UUID}.
*/
public static List<UUID> filterKnown(Collection<UUID> uuids) {
ArrayList<UUID> result = new ArrayList<>(uuids.size());
for (UUID uuid : uuids) {
if (!UNKNOWN_UUID.equals(uuid)) {
result.add(uuid);
}
}
return result;
}
}