diff --git a/src/sdk/main/src/ChunkedTransaction.cc b/src/sdk/main/src/ChunkedTransaction.cc index 315f05a88..8ea7af01a 100644 --- a/src/sdk/main/src/ChunkedTransaction.cc +++ b/src/sdk/main/src/ChunkedTransaction.cc @@ -415,6 +415,20 @@ ChunkedTransaction::ChunkedTransaction( return; } + // Security: Determine the expected transaction type from the first entry. + // All chunks must have the same transaction type to prevent transaction smuggling attacks. + proto::TransactionBody::DataCase expectedDataCase = proto::TransactionBody::DATA_NOT_SET; + if (!transactions.empty() && !transactions.cbegin()->second.empty()) + { + const proto::Transaction& firstTx = transactions.cbegin()->second.cbegin()->second; + proto::SignedTransaction signedTx; + signedTx.ParseFromArray(firstTx.signedtransactionbytes().data(), + static_cast(firstTx.signedtransactionbytes().size())); + proto::TransactionBody txBody; + txBody.ParseFromArray(signedTx.bodybytes().data(), static_cast(signedTx.bodybytes().size())); + expectedDataCase = txBody.data_case(); + } + // Go through each additional TransactionId and store its information. auto iter = transactions.cbegin(); std::advance(iter, 1); @@ -432,6 +446,23 @@ ChunkedTransaction::ChunkedTransaction( // ID, then no more account IDs will be in the map. if (!(accountId == Transaction::DUMMY_ACCOUNT_ID)) { + // Security: Validate that this chunk has the same transaction type as the first chunk. + // This is defense-in-depth against transaction smuggling attacks where an attacker + // might try to inject a different transaction type (e.g., CryptoTransfer) as a "chunk" + // of what appears to be a FileAppend or TopicMessageSubmit transaction. + proto::SignedTransaction signedTx; + signedTx.ParseFromArray(transaction.signedtransactionbytes().data(), + static_cast(transaction.signedtransactionbytes().size())); + proto::TransactionBody txBody; + txBody.ParseFromArray(signedTx.bodybytes().data(), static_cast(signedTx.bodybytes().size())); + + if (txBody.data_case() != expectedDataCase) + { + throw std::invalid_argument( + "ChunkedTransaction contains chunks with inconsistent transaction types. " + "All chunks must have the same transaction type to prevent transaction smuggling attacks."); + } + Transaction::addTransaction(transaction); } } diff --git a/src/sdk/main/src/Transaction.cc b/src/sdk/main/src/Transaction.cc index 7a316ede1..6a5a190d1 100644 --- a/src/sdk/main/src/Transaction.cc +++ b/src/sdk/main/src/Transaction.cc @@ -198,6 +198,7 @@ WrappedTransaction Transaction::fromBytes(const std::vector::fromBytes(const std::vector::fromBytes(const std::vector(tx.signedtransactionbytes().size())); txBody.ParseFromArray(signedTx.bodybytes().data(), static_cast(signedTx.bodybytes().size())); + // Security: Validate that all entries have the same transaction type. + // This is critical to prevent transaction smuggling where an attacker injects + // a hidden malicious transaction with a different type that gets signed but not displayed. + if (i == 0) + { + expectedDataCase = txBody.data_case(); + } + else if (txBody.data_case() != expectedDataCase) + { + throw std::invalid_argument( + "Transaction list contains entries with inconsistent transaction types. " + "All entries must have the same transaction type to prevent transaction smuggling attacks."); + } + std::string thisTxIdBytes = txBody.transactionid().SerializeAsString(); // Create a sanitized body (removing NodeAccountID) for comparison @@ -292,6 +308,23 @@ WrappedTransaction Transaction::fromBytes(const std::vector