Improve handling of badly-serialized data.

h/t @i-infra
This commit is contained in:
Greyson Parrelli 2021-10-02 15:57:07 -04:00
parent eb8de536e0
commit ed8538547f
3 changed files with 55 additions and 41 deletions

View file

@ -12,6 +12,7 @@ import androidx.lifecycle.LiveData;
import androidx.lifecycle.MutableLiveData; import androidx.lifecycle.MutableLiveData;
import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.InvalidProtocolBufferException;
import com.mobilecoin.lib.exceptions.SerializationException;
import org.signal.core.util.logging.Log; import org.signal.core.util.logging.Log;
import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper; import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper;
@ -30,6 +31,7 @@ import org.thoughtcrime.securesms.util.Base64;
import org.thoughtcrime.securesms.util.CursorUtil; import org.thoughtcrime.securesms.util.CursorUtil;
import org.thoughtcrime.securesms.util.SqlUtil; import org.thoughtcrime.securesms.util.SqlUtil;
import org.thoughtcrime.securesms.util.livedata.LiveDataUtil; import org.thoughtcrime.securesms.util.livedata.LiveDataUtil;
import org.whispersystems.signalservice.api.InvalidMessageStructureException;
import org.whispersystems.signalservice.api.payments.Money; import org.whispersystems.signalservice.api.payments.Money;
import java.util.Arrays; import java.util.Arrays;
@ -105,7 +107,7 @@ public final class PaymentDatabase extends Database {
@NonNull Money amount, @NonNull Money amount,
@NonNull Money fee, @NonNull Money fee,
@NonNull byte[] receipt) @NonNull byte[] receipt)
throws PublicKeyConflictException throws PublicKeyConflictException, SerializationException
{ {
create(uuid, fromRecipient, null, timestamp, 0, note, Direction.RECEIVED, State.SUBMITTED, amount, fee, null, receipt, null, false); create(uuid, fromRecipient, null, timestamp, 0, note, Direction.RECEIVED, State.SUBMITTED, amount, fee, null, receipt, null, false);
} }
@ -122,7 +124,9 @@ public final class PaymentDatabase extends Database {
create(uuid, toRecipient, publicAddress, timestamp, 0, note, Direction.SENT, State.INITIAL, amount, amount.toZero(), null, null, null, true); create(uuid, toRecipient, publicAddress, timestamp, 0, note, Direction.SENT, State.INITIAL, amount, amount.toZero(), null, null, null, true);
} catch (PublicKeyConflictException e) { } catch (PublicKeyConflictException e) {
Log.w(TAG, "Tried to create payment but the public key appears already in the database", e); Log.w(TAG, "Tried to create payment but the public key appears already in the database", e);
throw new AssertionError(e); throw new IllegalArgumentException(e);
} catch (SerializationException e) {
throw new IllegalArgumentException(e);
} }
} }
@ -142,6 +146,7 @@ public final class PaymentDatabase extends Database {
@NonNull Money fee, @NonNull Money fee,
@NonNull byte[] receipt, @NonNull byte[] receipt,
@NonNull PaymentMetaData metaData) @NonNull PaymentMetaData metaData)
throws SerializationException
{ {
try { try {
create(uuid, toRecipient, publicAddress, timestamp, blockIndex, note, Direction.SENT, State.SUCCESSFUL, amount, fee, null, receipt, metaData, true); create(uuid, toRecipient, publicAddress, timestamp, blockIndex, note, Direction.SENT, State.SUCCESSFUL, amount, fee, null, receipt, metaData, true);
@ -165,6 +170,8 @@ public final class PaymentDatabase extends Database {
} catch (PublicKeyConflictException e) { } catch (PublicKeyConflictException e) {
Log.w(TAG, "Tried to create payment but the public key appears already in the database", e); Log.w(TAG, "Tried to create payment but the public key appears already in the database", e);
throw new AssertionError(e); throw new AssertionError(e);
} catch (SerializationException e) {
throw new IllegalArgumentException(e);
} }
} }
@ -183,7 +190,7 @@ public final class PaymentDatabase extends Database {
@Nullable byte[] receipt, @Nullable byte[] receipt,
@Nullable PaymentMetaData metaData, @Nullable PaymentMetaData metaData,
boolean seen) boolean seen)
throws PublicKeyConflictException throws PublicKeyConflictException, SerializationException
{ {
if (recipientId == null && publicAddress == null) { if (recipientId == null && publicAddress == null) {
throw new AssertionError(); throw new AssertionError();
@ -403,8 +410,12 @@ public final class PaymentDatabase extends Database {
values.put(STATE, State.SUBMITTED.serialize()); values.put(STATE, State.SUBMITTED.serialize());
values.put(TRANSACTION, transaction); values.put(TRANSACTION, transaction);
values.put(RECEIPT, receipt); values.put(RECEIPT, receipt);
try {
values.put(PUBLIC_KEY, Base64.encodeBytes(PaymentMetaDataUtil.receiptPublic(PaymentMetaDataUtil.fromReceipt(receipt)))); values.put(PUBLIC_KEY, Base64.encodeBytes(PaymentMetaDataUtil.receiptPublic(PaymentMetaDataUtil.fromReceipt(receipt))));
values.put(META_DATA, PaymentMetaDataUtil.fromReceiptAndTransaction(receipt, transaction).toByteArray()); values.put(META_DATA, PaymentMetaDataUtil.fromReceiptAndTransaction(receipt, transaction).toByteArray());
} catch (SerializationException e) {
throw new IllegalArgumentException(e);
}
values.put(FEE, CryptoValueUtil.moneyToCryptoValue(fee).toByteArray()); values.put(FEE, CryptoValueUtil.moneyToCryptoValue(fee).toByteArray());
database.beginTransaction(); database.beginTransaction();

View file

@ -23,11 +23,11 @@ public final class PaymentMetaDataUtil {
try { try {
return PaymentMetaData.parseFrom(requireBlob); return PaymentMetaData.parseFrom(requireBlob);
} catch (InvalidProtocolBufferException e) { } catch (InvalidProtocolBufferException e) {
throw new AssertionError(e); throw new IllegalStateException(e);
} }
} }
public static @NonNull PaymentMetaData fromReceipt(@Nullable byte[] receipt) { public static @NonNull PaymentMetaData fromReceipt(@Nullable byte[] receipt) throws SerializationException {
PaymentMetaData.MobileCoinTxoIdentification.Builder builder = PaymentMetaData.MobileCoinTxoIdentification.newBuilder(); PaymentMetaData.MobileCoinTxoIdentification.Builder builder = PaymentMetaData.MobileCoinTxoIdentification.newBuilder();
if (receipt != null) { if (receipt != null) {
@ -46,7 +46,7 @@ public final class PaymentMetaDataUtil {
return PaymentMetaData.newBuilder().setMobileCoinTxoIdentification(builder).build(); return PaymentMetaData.newBuilder().setMobileCoinTxoIdentification(builder).build();
} }
public static @NonNull PaymentMetaData fromReceiptAndTransaction(@Nullable byte[] receipt, @Nullable byte[] transaction) { public static @NonNull PaymentMetaData fromReceiptAndTransaction(@Nullable byte[] receipt, @Nullable byte[] transaction) throws SerializationException {
PaymentMetaData.MobileCoinTxoIdentification.Builder builder = PaymentMetaData.MobileCoinTxoIdentification.newBuilder(); PaymentMetaData.MobileCoinTxoIdentification.Builder builder = PaymentMetaData.MobileCoinTxoIdentification.newBuilder();
if (transaction != null) { if (transaction != null) {
@ -58,17 +58,12 @@ public final class PaymentMetaDataUtil {
return PaymentMetaData.newBuilder().setMobileCoinTxoIdentification(builder).build(); return PaymentMetaData.newBuilder().setMobileCoinTxoIdentification(builder).build();
} }
private static void addReceiptData(@NonNull byte[] receipt, PaymentMetaData.MobileCoinTxoIdentification.Builder builder) { private static void addReceiptData(@NonNull byte[] receipt, PaymentMetaData.MobileCoinTxoIdentification.Builder builder) throws SerializationException {
try {
RistrettoPublic publicKey = Receipt.fromBytes(receipt).getPublicKey(); RistrettoPublic publicKey = Receipt.fromBytes(receipt).getPublicKey();
addPublicKey(builder, publicKey); addPublicKey(builder, publicKey);
} catch (SerializationException e) {
throw new AssertionError(e);
}
} }
private static void addTransactionData(@NonNull byte[] transactionBytes, PaymentMetaData.MobileCoinTxoIdentification.Builder builder) { private static void addTransactionData(@NonNull byte[] transactionBytes, PaymentMetaData.MobileCoinTxoIdentification.Builder builder) throws SerializationException {
try {
Transaction transaction = Transaction.fromBytes(transactionBytes); Transaction transaction = Transaction.fromBytes(transactionBytes);
Set<KeyImage> keyImages = transaction.getKeyImages(); Set<KeyImage> keyImages = transaction.getKeyImages();
for (KeyImage keyImage : keyImages) { for (KeyImage keyImage : keyImages) {
@ -77,9 +72,6 @@ public final class PaymentMetaDataUtil {
for (RistrettoPublic publicKey : transaction.getOutputPublicKeys()) { for (RistrettoPublic publicKey : transaction.getOutputPublicKeys()) {
addPublicKey(builder, publicKey); addPublicKey(builder, publicKey);
} }
} catch (SerializationException e) {
throw new AssertionError(e);
}
} }
private static void addPublicKey(@NonNull PaymentMetaData.MobileCoinTxoIdentification.Builder builder, @NonNull RistrettoPublic publicKey) { private static void addPublicKey(@NonNull PaymentMetaData.MobileCoinTxoIdentification.Builder builder, @NonNull RistrettoPublic publicKey) {

View file

@ -10,6 +10,7 @@ import androidx.annotation.Nullable;
import com.annimon.stream.Collectors; import com.annimon.stream.Collectors;
import com.annimon.stream.Stream; import com.annimon.stream.Stream;
import com.mobilecoin.lib.exceptions.SerializationException;
import org.signal.core.util.logging.Log; import org.signal.core.util.logging.Log;
import org.signal.ringrtc.CallId; import org.signal.ringrtc.CallId;
@ -309,7 +310,7 @@ public final class MessageContentProcessor {
else if (syncMessage.getBlockedList().isPresent()) handleSynchronizeBlockedListMessage(syncMessage.getBlockedList().get()); else if (syncMessage.getBlockedList().isPresent()) handleSynchronizeBlockedListMessage(syncMessage.getBlockedList().get());
else if (syncMessage.getFetchType().isPresent()) handleSynchronizeFetchMessage(syncMessage.getFetchType().get()); else if (syncMessage.getFetchType().isPresent()) handleSynchronizeFetchMessage(syncMessage.getFetchType().get());
else if (syncMessage.getMessageRequestResponse().isPresent()) handleSynchronizeMessageRequestResponse(syncMessage.getMessageRequestResponse().get()); else if (syncMessage.getMessageRequestResponse().isPresent()) handleSynchronizeMessageRequestResponse(syncMessage.getMessageRequestResponse().get());
else if (syncMessage.getOutgoingPaymentMessage().isPresent()) handleSynchronizeOutgoingPayment(syncMessage.getOutgoingPaymentMessage().get()); else if (syncMessage.getOutgoingPaymentMessage().isPresent()) handleSynchronizeOutgoingPayment(content, syncMessage.getOutgoingPaymentMessage().get());
else warn(String.valueOf(content.getTimestamp()), "Contains no known sync types..."); else warn(String.valueOf(content.getTimestamp()), "Contains no known sync types...");
} else if (content.getCallMessage().isPresent()) { } else if (content.getCallMessage().isPresent()) {
log(String.valueOf(content.getTimestamp()), "Got call message..."); log(String.valueOf(content.getTimestamp()), "Got call message...");
@ -410,6 +411,8 @@ public final class MessageContentProcessor {
} catch (PaymentDatabase.PublicKeyConflictException e) { } catch (PaymentDatabase.PublicKeyConflictException e) {
warn(content.getTimestamp(), "Ignoring payment with public key already in database"); warn(content.getTimestamp(), "Ignoring payment with public key already in database");
return; return;
} catch (SerializationException e) {
warn(content.getTimestamp(), "Ignoring payment with bad data.", e);
} }
ApplicationDependencies.getJobManager() ApplicationDependencies.getJobManager()
@ -1016,7 +1019,7 @@ public final class MessageContentProcessor {
} }
} }
private void handleSynchronizeOutgoingPayment(@NonNull OutgoingPaymentMessage outgoingPaymentMessage) { private void handleSynchronizeOutgoingPayment(@NonNull SignalServiceContent content, @NonNull OutgoingPaymentMessage outgoingPaymentMessage) {
RecipientId recipientId = outgoingPaymentMessage.getRecipient() RecipientId recipientId = outgoingPaymentMessage.getRecipient()
.transform(uuid -> RecipientId.from(uuid, null)) .transform(uuid -> RecipientId.from(uuid, null))
.orNull(); .orNull();
@ -1027,12 +1030,13 @@ public final class MessageContentProcessor {
Optional<MobileCoinPublicAddress> address = outgoingPaymentMessage.getAddress().transform(MobileCoinPublicAddress::fromBytes); Optional<MobileCoinPublicAddress> address = outgoingPaymentMessage.getAddress().transform(MobileCoinPublicAddress::fromBytes);
if (!address.isPresent() && recipientId == null) { if (!address.isPresent() && recipientId == null) {
log("Inserting defrag"); log(content.getTimestamp(), "Inserting defrag");
address = Optional.of(ApplicationDependencies.getPayments().getWallet().getMobileCoinPublicAddress()); address = Optional.of(ApplicationDependencies.getPayments().getWallet().getMobileCoinPublicAddress());
recipientId = Recipient.self().getId(); recipientId = Recipient.self().getId();
} }
UUID uuid = UUID.randomUUID(); UUID uuid = UUID.randomUUID();
try {
DatabaseFactory.getPaymentDatabase(context) DatabaseFactory.getPaymentDatabase(context)
.createSuccessfulPayment(uuid, .createSuccessfulPayment(uuid,
recipientId, recipientId,
@ -1044,6 +1048,9 @@ public final class MessageContentProcessor {
outgoingPaymentMessage.getFee(), outgoingPaymentMessage.getFee(),
outgoingPaymentMessage.getReceipt().toByteArray(), outgoingPaymentMessage.getReceipt().toByteArray(),
PaymentMetaDataUtil.fromKeysAndImages(outgoingPaymentMessage.getPublicKeys(), outgoingPaymentMessage.getKeyImages())); PaymentMetaDataUtil.fromKeysAndImages(outgoingPaymentMessage.getPublicKeys(), outgoingPaymentMessage.getKeyImages()));
} catch (SerializationException e) {
warn(content.getTimestamp(), "Ignoring synchronized outgoing payment with bad data.", e);
}
log("Inserted synchronized payment " + uuid); log("Inserted synchronized payment " + uuid);
} }
@ -2254,6 +2261,10 @@ public final class MessageContentProcessor {
warn(String.valueOf(timestamp), message); warn(String.valueOf(timestamp), message);
} }
protected void warn(long timestamp, @NonNull String message, @Nullable Throwable t) {
warn(String.valueOf(timestamp), message, t);
}
protected void warn(@NonNull String message, @Nullable Throwable t) { protected void warn(@NonNull String message, @Nullable Throwable t) {
warn("", message, t); warn("", message, t);
} }