From 28d86886bd508e4fa03f44db06806ece51ff9277 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 25 Aug 2021 13:34:29 -0400 Subject: [PATCH] Update handling of invalid unknown fields. --- .../api/storage/SignalAccountRecord.java | 10 +++++++- .../api/storage/SignalContactRecord.java | 10 +++++++- .../api/storage/SignalGroupV1Record.java | 10 +++++++- .../api/storage/SignalGroupV2Record.java | 10 +++++++- .../signalservice/api/util/ProtoUtil.java | 25 +++++++++---------- 5 files changed, 48 insertions(+), 17 deletions(-) diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalAccountRecord.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalAccountRecord.java index 1f284026c..340bf53a3 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalAccountRecord.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalAccountRecord.java @@ -1,6 +1,7 @@ package org.whispersystems.signalservice.api.storage; import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; import org.whispersystems.libsignal.logging.Log; import org.whispersystems.libsignal.util.guava.Optional; @@ -19,6 +20,8 @@ import java.util.Objects; public final class SignalAccountRecord implements SignalRecord { + private static final String TAG = SignalAccountRecord.class.getSimpleName(); + private final StorageId id; private final AccountRecord proto; private final boolean hasUnknownFields; @@ -489,7 +492,12 @@ public final class SignalAccountRecord implements SignalRecord { AccountRecord proto = builder.build(); if (unknownFields != null) { - proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + try { + proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + } catch (InvalidProtocolBufferException e) { + Log.w(TAG, "Failed to combine unknown fields!", e); + throw new IllegalStateException(e); + } } return new SignalAccountRecord(id, proto); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java index 0ddca30a1..4d693d9c5 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java @@ -1,7 +1,9 @@ package org.whispersystems.signalservice.api.storage; import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; +import org.whispersystems.libsignal.logging.Log; import org.whispersystems.libsignal.util.guava.Optional; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.util.OptionalUtil; @@ -17,6 +19,8 @@ import java.util.Objects; public final class SignalContactRecord implements SignalRecord { + private static final String TAG = SignalContactRecord.class.getSimpleName(); + private final StorageId id; private final ContactRecord proto; private final boolean hasUnknownFields; @@ -275,7 +279,11 @@ public final class SignalContactRecord implements SignalRecord { ContactRecord proto = builder.build(); if (unknownFields != null) { - proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + try { + proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + } catch (InvalidProtocolBufferException e) { + Log.w(TAG, "Failed to combine unknown fields!", e); + } } return new SignalContactRecord(id, proto); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV1Record.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV1Record.java index 15e92cb42..b150cb355 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV1Record.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV1Record.java @@ -1,7 +1,9 @@ package org.whispersystems.signalservice.api.storage; import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; +import org.whispersystems.libsignal.logging.Log; import org.whispersystems.signalservice.api.util.ProtoUtil; import org.whispersystems.signalservice.internal.storage.protos.GroupV1Record; @@ -12,6 +14,8 @@ import java.util.Objects; public final class SignalGroupV1Record implements SignalRecord { + private static final String TAG = SignalGroupV1Record.class.getSimpleName(); + private final StorageId id; private final GroupV1Record proto; private final byte[] groupId; @@ -175,7 +179,11 @@ public final class SignalGroupV1Record implements SignalRecord { GroupV1Record proto = builder.build(); if (unknownFields != null) { - proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + try { + proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + } catch (InvalidProtocolBufferException e) { + Log.w(TAG, "Failed to combine unknown fields!", e); + } } return new SignalGroupV1Record(id, proto); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java index 91adb9aa8..0d7dcd093 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java @@ -1,9 +1,11 @@ package org.whispersystems.signalservice.api.storage; import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; import org.signal.zkgroup.InvalidInputException; import org.signal.zkgroup.groups.GroupMasterKey; +import org.whispersystems.libsignal.logging.Log; import org.whispersystems.signalservice.api.util.ProtoUtil; import org.whispersystems.signalservice.internal.storage.protos.GroupV2Record; @@ -14,6 +16,8 @@ import java.util.Objects; public final class SignalGroupV2Record implements SignalRecord { + private static final String TAG = SignalGroupV2Record.class.getSimpleName(); + private final StorageId id; private final GroupV2Record proto; private final byte[] masterKey; @@ -190,7 +194,11 @@ public final class SignalGroupV2Record implements SignalRecord { GroupV2Record proto = builder.build(); if (unknownFields != null) { - proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + try { + proto = ProtoUtil.combineWithUnknownFields(proto, unknownFields); + } catch (InvalidProtocolBufferException e) { + Log.w(TAG, "Failed to combine unknown fields!", e); + } } return new SignalGroupV2Record(id, proto); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/util/ProtoUtil.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/util/ProtoUtil.java index 306b8088e..c062d61c6 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/util/ProtoUtil.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/util/ProtoUtil.java @@ -7,6 +7,7 @@ import com.google.protobuf.UnknownFieldSetLite; import org.whispersystems.libsignal.logging.Log; import org.whispersystems.libsignal.util.ByteUtil; +import org.whispersystems.signalservice.api.InvalidMessageStructureException; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -60,25 +61,23 @@ public final class ProtoUtil { * acknowledged. */ @SuppressWarnings({"rawtypes", "unchecked"}) - public static Proto combineWithUnknownFields(Proto proto, byte[] serializedWithUnknownFields) { + public static Proto combineWithUnknownFields(Proto proto, byte[] serializedWithUnknownFields) + throws InvalidProtocolBufferException + { if (serializedWithUnknownFields == null) { return proto; } - try { - Proto protoWithUnknownFields = (Proto) proto.getParserForType().parseFrom(serializedWithUnknownFields); - byte[] unknownFields = getUnknownFields(protoWithUnknownFields); + Proto protoWithUnknownFields = (Proto) proto.getParserForType().parseFrom(serializedWithUnknownFields); + byte[] unknownFields = getUnknownFields(protoWithUnknownFields); - if (unknownFields == null) { - return proto; - } - - byte[] combined = ByteUtil.combine(proto.toByteArray(), unknownFields); - - return (Proto) proto.getParserForType().parseFrom(combined); - } catch (InvalidProtocolBufferException e) { - throw new IllegalArgumentException(); + if (unknownFields == null) { + return proto; } + + byte[] combined = ByteUtil.combine(proto.toByteArray(), unknownFields); + + return (Proto) proto.getParserForType().parseFrom(combined); } @SuppressWarnings("rawtypes")