Skip to content

Commit a40b58b

Browse files
rwalworthrwalworth
authored
fix: strengthen TransactionList validation in fromBytes (#589)
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
1 parent 563b9aa commit a40b58b

File tree

3 files changed

+210
-1
lines changed

3 files changed

+210
-1
lines changed

‎Sources/Hiero/Topic/TopicMessageSubmitTransaction.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public final class TopicMessageSubmitTransaction: ChunkedTransaction {
2828
var message: Data = first.message
2929
var largestChunkSize = max(first.message.count, 1)
3030

31-
// note: no other SDK checks for correctness here... so let's not do it here either?
31+
// Chunk structure (count, sequential numbers) is validated in Transaction.fromBytes.
3232
for item in iter {
3333
largestChunkSize = max(largestChunkSize, item.message.count)
3434
message.append(item.message)

‎Sources/Hiero/Transaction.swift‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,43 @@ public class Transaction: ValidateChecksums {
478478

479479
}
480480

481+
// SECURITY: Only chunked transaction types (consensusSubmitMessage, fileAppend) legitimately
482+
// have multiple TransactionId groups. Reject all other types unconditionally to prevent
483+
// cross-group body forgery attacks where a hidden group carries a different operation.
484+
if sources.chunksCount > 1 {
485+
switch transactionBodies[0].data {
486+
case .consensusSubmitMessage, .fileAppend:
487+
break
488+
default:
489+
throw HError.fromProtobuf("non-chunked transaction must not have multiple transaction ID groups")
490+
}
491+
}
492+
493+
// For consensusSubmitMessage with chunkInfo, validate that the actual group count matches
494+
// chunkInfo.total and that chunk numbers are sequential. This bounds the attack surface for
495+
// chunked types: post-signing the body bytes (including chunkInfo.total) are locked by the
496+
// signature, so an attacker must set chunkInfo.total == actual group count upfront, making
497+
// any injected groups visible in the victim's reconstructed message via SDK getters.
498+
if case .consensusSubmitMessage(let msg) = transactionBodies[0].data, msg.hasChunkInfo {
499+
let declaredTotal = Int(msg.chunkInfo.total)
500+
guard sources.chunksCount == declaredTotal else {
501+
throw HError.fromProtobuf(
502+
"chunked transaction has \(sources.chunksCount) groups but chunkInfo.total declares \(declaredTotal)"
503+
)
504+
}
505+
for (chunkIndex, chunk) in sources.chunks.enumerated() {
506+
let chunkBody = transactionBodies[chunk.range.lowerBound]
507+
if case .consensusSubmitMessage(let chunkMsg) = chunkBody.data, chunkMsg.hasChunkInfo {
508+
guard chunkMsg.chunkInfo.number == Int32(chunkIndex + 1) else {
509+
throw HError.fromProtobuf(
510+
"chunk number out of order at group \(chunkIndex): "
511+
+ "expected \(chunkIndex + 1), got \(chunkMsg.chunkInfo.number)"
512+
)
513+
}
514+
}
515+
}
516+
}
517+
481518
let transactionData =
482519
try sources
483520
.chunks

‎Tests/HieroUnitTests/ChunkedTransactionUnitTests.swift‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,176 @@ internal final class ChunkedTransactionUnitTests: HieroUnitTestCase {
3232
XCTAssertEqual(transaction.message, "Hello, world!".data(using: .utf8)!)
3333
XCTAssertEqual(transaction.transactionId, transactionId)
3434
}
35+
36+
// MARK: - Cross-group forgery rejection tests
37+
38+
/// Non-chunked types (e.g. CryptoTransfer) with multiple TransactionId groups must be rejected,
39+
/// even when the bodies are identical across groups.
40+
internal func test_fromBytes_rejectMultiGroupNonChunked() throws {
41+
let txId1 = TransactionId.withValidStart(
42+
AccountId(num: 100), Timestamp(seconds: 1_554_158_542, subSecondNanos: 0))
43+
let txId2 = TransactionId.withValidStart(
44+
AccountId(num: 100), Timestamp(seconds: 1_554_158_543, subSecondNanos: 0))
45+
46+
let transfer = Proto_CryptoTransferTransactionBody.with { proto in
47+
proto.transfers = Proto_TransferList.with { list in
48+
list.accountAmounts = [
49+
Proto_AccountAmount.with { amt in
50+
amt.accountID = AccountId(num: 100).toProtobuf()
51+
amt.amount = -100
52+
},
53+
Proto_AccountAmount.with { amt in
54+
amt.accountID = AccountId(num: 200).toProtobuf()
55+
amt.amount = 100
56+
},
57+
]
58+
}
59+
}
60+
61+
let tx1 = try makeSignedTx(txId: txId1) { $0.cryptoTransfer = transfer }
62+
let tx2 = try makeSignedTx(txId: txId2) { $0.cryptoTransfer = transfer }
63+
64+
var list = Proto_TransactionList()
65+
list.transactionList = [tx1, tx2]
66+
let bytes = try list.serializedData()
67+
68+
XCTAssertThrowsError(try Transaction.fromBytes(bytes))
69+
}
70+
71+
/// A consensusSubmitMessage list with 2 actual groups but chunkInfo.total = 1 must be rejected.
72+
internal func test_fromBytes_rejectChunkCountMismatch_totalTooLow() throws {
73+
let txId1 = TransactionId.withValidStart(
74+
AccountId(num: 100), Timestamp(seconds: 1_554_158_542, subSecondNanos: 0))
75+
let txId2 = TransactionId.withValidStart(
76+
AccountId(num: 100), Timestamp(seconds: 1_554_158_543, subSecondNanos: 0))
77+
let topicId = TopicId(num: 1)
78+
79+
// Both groups declare total=1, but there are 2 actual groups.
80+
let tx1 = try makeSignedTx(txId: txId1) { body in
81+
body.consensusSubmitMessage = Proto_ConsensusSubmitMessageTransactionBody.with { proto in
82+
proto.topicID = topicId.toProtobuf()
83+
proto.message = Data("chunk1".utf8)
84+
proto.chunkInfo = Proto_ConsensusMessageChunkInfo.with { info in
85+
info.initialTransactionID = txId1.toProtobuf()
86+
info.total = 1
87+
info.number = 1
88+
}
89+
}
90+
}
91+
let tx2 = try makeSignedTx(txId: txId2) { body in
92+
body.consensusSubmitMessage = Proto_ConsensusSubmitMessageTransactionBody.with { proto in
93+
proto.topicID = topicId.toProtobuf()
94+
proto.message = Data("chunk2".utf8)
95+
proto.chunkInfo = Proto_ConsensusMessageChunkInfo.with { info in
96+
info.initialTransactionID = txId1.toProtobuf()
97+
info.total = 1
98+
info.number = 1
99+
}
100+
}
101+
}
102+
103+
var list = Proto_TransactionList()
104+
list.transactionList = [tx1, tx2]
105+
let bytes = try list.serializedData()
106+
107+
XCTAssertThrowsError(try Transaction.fromBytes(bytes))
108+
}
109+
110+
/// A consensusSubmitMessage list with 2 actual groups but chunkInfo.total = 3 must be rejected.
111+
internal func test_fromBytes_rejectChunkCountMismatch_totalTooHigh() throws {
112+
let txId1 = TransactionId.withValidStart(
113+
AccountId(num: 100), Timestamp(seconds: 1_554_158_542, subSecondNanos: 0))
114+
let txId2 = TransactionId.withValidStart(
115+
AccountId(num: 100), Timestamp(seconds: 1_554_158_543, subSecondNanos: 0))
116+
let topicId = TopicId(num: 1)
117+
118+
// Both groups declare total=3, but there are only 2 actual groups.
119+
let tx1 = try makeSignedTx(txId: txId1) { body in
120+
body.consensusSubmitMessage = Proto_ConsensusSubmitMessageTransactionBody.with { proto in
121+
proto.topicID = topicId.toProtobuf()
122+
proto.message = Data("chunk1".utf8)
123+
proto.chunkInfo = Proto_ConsensusMessageChunkInfo.with { info in
124+
info.initialTransactionID = txId1.toProtobuf()
125+
info.total = 3
126+
info.number = 1
127+
}
128+
}
129+
}
130+
let tx2 = try makeSignedTx(txId: txId2) { body in
131+
body.consensusSubmitMessage = Proto_ConsensusSubmitMessageTransactionBody.with { proto in
132+
proto.topicID = topicId.toProtobuf()
133+
proto.message = Data("chunk2".utf8)
134+
proto.chunkInfo = Proto_ConsensusMessageChunkInfo.with { info in
135+
info.initialTransactionID = txId1.toProtobuf()
136+
info.total = 3
137+
info.number = 2
138+
}
139+
}
140+
}
141+
142+
var list = Proto_TransactionList()
143+
list.transactionList = [tx1, tx2]
144+
let bytes = try list.serializedData()
145+
146+
XCTAssertThrowsError(try Transaction.fromBytes(bytes))
147+
}
148+
149+
/// A consensusSubmitMessage list with swapped chunk numbers must be rejected.
150+
internal func test_fromBytes_rejectOutOfOrderChunkNumbers() throws {
151+
let txId1 = TransactionId.withValidStart(
152+
AccountId(num: 100), Timestamp(seconds: 1_554_158_542, subSecondNanos: 0))
153+
let txId2 = TransactionId.withValidStart(
154+
AccountId(num: 100), Timestamp(seconds: 1_554_158_543, subSecondNanos: 0))
155+
let topicId = TopicId(num: 1)
156+
157+
// Group appearing first declares number=2; group appearing second declares number=1.
158+
let tx1 = try makeSignedTx(txId: txId1) { body in
159+
body.consensusSubmitMessage = Proto_ConsensusSubmitMessageTransactionBody.with { proto in
160+
proto.topicID = topicId.toProtobuf()
161+
proto.message = Data("chunk2".utf8)
162+
proto.chunkInfo = Proto_ConsensusMessageChunkInfo.with { info in
163+
info.initialTransactionID = txId1.toProtobuf()
164+
info.total = 2
165+
info.number = 2
166+
}
167+
}
168+
}
169+
let tx2 = try makeSignedTx(txId: txId2) { body in
170+
body.consensusSubmitMessage = Proto_ConsensusSubmitMessageTransactionBody.with { proto in
171+
proto.topicID = topicId.toProtobuf()
172+
proto.message = Data("chunk1".utf8)
173+
proto.chunkInfo = Proto_ConsensusMessageChunkInfo.with { info in
174+
info.initialTransactionID = txId1.toProtobuf()
175+
info.total = 2
176+
info.number = 1
177+
}
178+
}
179+
}
180+
181+
var list = Proto_TransactionList()
182+
list.transactionList = [tx1, tx2]
183+
let bytes = try list.serializedData()
184+
185+
XCTAssertThrowsError(try Transaction.fromBytes(bytes))
186+
}
187+
188+
// MARK: - Helpers
189+
190+
private func makeSignedTx(
191+
txId: TransactionId,
192+
nodeId: AccountId = AccountId(num: 3),
193+
configure: (inout Proto_TransactionBody) -> Void
194+
) throws -> Proto_Transaction {
195+
var body = Proto_TransactionBody()
196+
body.transactionID = txId.toProtobuf()
197+
body.nodeAccountID = nodeId.toProtobuf()
198+
configure(&body)
199+
200+
var signedTx = Proto_SignedTransaction()
201+
signedTx.bodyBytes = try body.serializedData()
202+
203+
var tx = Proto_Transaction()
204+
tx.signedTransactionBytes = try signedTx.serializedData()
205+
return tx
206+
}
35207
}

0 commit comments

Comments
 (0)