From 570ac25aa92e8cfca98510ec7c81b95f6afb1662 Mon Sep 17 00:00:00 2001 From: wb Date: Fri, 24 Jul 2026 19:13:16 +0800 Subject: [PATCH] fix(net): discard unknown fields from transient messages Transient P2P and backup control messages retained attacker-supplied protobuf unknown fields after parsing, increasing their in-memory footprint without affecting message semantics. Parse non-persistent control messages with DiscardUnknownFieldsParser while preserving the exact serialized bytes of persisted and consensus-critical messages. --- .../backup/message/KeepAliveMessage.java | 4 +- .../net/message/adv/InventoryMessage.java | 3 +- .../net/message/base/DisconnectMessage.java | 9 +- .../net/message/handshake/HelloMessage.java | 7 +- .../message/sync/BlockInventoryMessage.java | 4 +- .../message/sync/ChainInventoryMessage.java | 4 +- .../common/backup/KeepAliveMessageTest.java | 24 +++++ .../core/net/MessageUnknownFieldTest.java | 97 +++++++++++++++++++ 8 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/net/MessageUnknownFieldTest.java diff --git a/framework/src/main/java/org/tron/common/backup/message/KeepAliveMessage.java b/framework/src/main/java/org/tron/common/backup/message/KeepAliveMessage.java index fca63451107..ca13b574a2b 100644 --- a/framework/src/main/java/org/tron/common/backup/message/KeepAliveMessage.java +++ b/framework/src/main/java/org/tron/common/backup/message/KeepAliveMessage.java @@ -2,6 +2,7 @@ import static org.tron.common.backup.message.UdpMessageTypeEnum.BACKUP_KEEP_ALIVE; +import com.google.protobuf.DiscardUnknownFieldsParser; import org.tron.p2p.discover.Node; import org.tron.protos.Discover; @@ -11,7 +12,8 @@ public class KeepAliveMessage extends Message { public KeepAliveMessage(byte[] data) throws Exception { super(BACKUP_KEEP_ALIVE, data); - backupMessage = Discover.BackupMessage.parseFrom(data); + backupMessage = DiscardUnknownFieldsParser.wrap(Discover.BackupMessage.parser()) + .parseFrom(data); } public KeepAliveMessage(boolean flag, int priority) { diff --git a/framework/src/main/java/org/tron/core/net/message/adv/InventoryMessage.java b/framework/src/main/java/org/tron/core/net/message/adv/InventoryMessage.java index d0991a9dcd3..e879a21a735 100644 --- a/framework/src/main/java/org/tron/core/net/message/adv/InventoryMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/adv/InventoryMessage.java @@ -1,5 +1,6 @@ package org.tron.core.net.message.adv; +import com.google.protobuf.DiscardUnknownFieldsParser; import java.util.Deque; import java.util.LinkedList; import java.util.List; @@ -19,7 +20,7 @@ public class InventoryMessage extends TronMessage { public InventoryMessage(byte[] data) throws Exception { super(data); this.type = MessageTypes.INVENTORY.asByte(); - this.inv = Protocol.Inventory.parseFrom(data); + this.inv = DiscardUnknownFieldsParser.wrap(Protocol.Inventory.parser()).parseFrom(data); } public InventoryMessage(Inventory inv) { diff --git a/framework/src/main/java/org/tron/core/net/message/base/DisconnectMessage.java b/framework/src/main/java/org/tron/core/net/message/base/DisconnectMessage.java index c8d0d2485ff..5c804314c64 100755 --- a/framework/src/main/java/org/tron/core/net/message/base/DisconnectMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/base/DisconnectMessage.java @@ -1,5 +1,6 @@ package org.tron.core.net.message.base; +import com.google.protobuf.DiscardUnknownFieldsParser; import org.tron.core.net.message.MessageTypes; import org.tron.core.net.message.TronMessage; import org.tron.protos.Protocol; @@ -11,12 +12,14 @@ public class DisconnectMessage extends TronMessage { public DisconnectMessage(byte type, byte[] rawData) throws Exception { super(type, rawData); - this.disconnectMessage = Protocol.DisconnectMessage.parseFrom(this.data); + this.disconnectMessage = DiscardUnknownFieldsParser.wrap(Protocol.DisconnectMessage.parser()) + .parseFrom(this.data); } public DisconnectMessage(byte[] data) throws Exception { super(MessageTypes.P2P_DISCONNECT.asByte(), data); - this.disconnectMessage = Protocol.DisconnectMessage.parseFrom(data); + this.disconnectMessage = DiscardUnknownFieldsParser.wrap(Protocol.DisconnectMessage.parser()) + .parseFrom(data); } public DisconnectMessage(ReasonCode reasonCode) { @@ -46,4 +49,4 @@ public String toString() { public Class getAnswerMessage() { return null; } -} \ No newline at end of file +} diff --git a/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java b/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java index 68123c93db6..c5fa25533ae 100755 --- a/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java @@ -1,6 +1,7 @@ package org.tron.core.net.message.handshake; import com.google.protobuf.ByteString; +import com.google.protobuf.DiscardUnknownFieldsParser; import lombok.Getter; import org.apache.commons.lang3.StringUtils; import org.tron.common.utils.ByteArray; @@ -25,12 +26,14 @@ public class HelloMessage extends TronMessage { public HelloMessage(byte type, byte[] rawData) throws Exception { super(type, rawData); - this.helloMessage = Protocol.HelloMessage.parseFrom(rawData); + this.helloMessage = DiscardUnknownFieldsParser.wrap(Protocol.HelloMessage.parser()) + .parseFrom(rawData); } public HelloMessage(byte[] data) throws Exception { super(MessageTypes.P2P_HELLO.asByte(), data); - this.helloMessage = Protocol.HelloMessage.parseFrom(data); + this.helloMessage = DiscardUnknownFieldsParser.wrap(Protocol.HelloMessage.parser()) + .parseFrom(data); } public HelloMessage(Node from, long timestamp, ChainBaseManager chainBaseManager) { diff --git a/framework/src/main/java/org/tron/core/net/message/sync/BlockInventoryMessage.java b/framework/src/main/java/org/tron/core/net/message/sync/BlockInventoryMessage.java index 53a736dcb90..c91e6c37a1c 100644 --- a/framework/src/main/java/org/tron/core/net/message/sync/BlockInventoryMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/sync/BlockInventoryMessage.java @@ -1,5 +1,6 @@ package org.tron.core.net.message.sync; +import com.google.protobuf.DiscardUnknownFieldsParser; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -16,7 +17,8 @@ public class BlockInventoryMessage extends TronMessage { public BlockInventoryMessage(byte[] data) throws Exception { super(data); this.type = MessageTypes.BLOCK_INVENTORY.asByte(); - this.blockInventory = Protocol.BlockInventory.parseFrom(data); + this.blockInventory = DiscardUnknownFieldsParser.wrap(Protocol.BlockInventory.parser()) + .parseFrom(data); } public BlockInventoryMessage(List blockIds, BlockInventory.Type type) { diff --git a/framework/src/main/java/org/tron/core/net/message/sync/ChainInventoryMessage.java b/framework/src/main/java/org/tron/core/net/message/sync/ChainInventoryMessage.java index 610a5ff2f11..9cabca15ed4 100644 --- a/framework/src/main/java/org/tron/core/net/message/sync/ChainInventoryMessage.java +++ b/framework/src/main/java/org/tron/core/net/message/sync/ChainInventoryMessage.java @@ -1,5 +1,6 @@ package org.tron.core.net.message.sync; +import com.google.protobuf.DiscardUnknownFieldsParser; import java.util.ArrayList; import java.util.Deque; import java.util.LinkedList; @@ -18,7 +19,8 @@ public class ChainInventoryMessage extends TronMessage { public ChainInventoryMessage(byte[] data) throws Exception { super(data); this.type = MessageTypes.BLOCK_CHAIN_INVENTORY.asByte(); - chainInventory = Protocol.ChainInventory.parseFrom(data); + chainInventory = DiscardUnknownFieldsParser.wrap(Protocol.ChainInventory.parser()) + .parseFrom(data); } public ChainInventoryMessage(List blockIds, Long remainNum) { diff --git a/framework/src/test/java/org/tron/common/backup/KeepAliveMessageTest.java b/framework/src/test/java/org/tron/common/backup/KeepAliveMessageTest.java index 67658331913..483fd6290b1 100644 --- a/framework/src/test/java/org/tron/common/backup/KeepAliveMessageTest.java +++ b/framework/src/test/java/org/tron/common/backup/KeepAliveMessageTest.java @@ -2,6 +2,8 @@ import static org.tron.common.backup.message.UdpMessageTypeEnum.BACKUP_KEEP_ALIVE; +import com.google.protobuf.ByteString; +import com.google.protobuf.UnknownFieldSet; import org.junit.Assert; import org.junit.Test; import org.tron.common.backup.message.KeepAliveMessage; @@ -9,6 +11,28 @@ public class KeepAliveMessageTest { + // A field number not used by BackupMessage, so it always parses as an unknown field. + private static final int UNKNOWN_FIELD_NUMBER = 1000; + + @Test + public void discardsUnknownFields() throws Exception { + Discover.BackupMessage backupMessage = Discover.BackupMessage.newBuilder() + .setFlag(true).setPriority(10).build(); + UnknownFieldSet unknown = UnknownFieldSet.newBuilder() + .addField(UNKNOWN_FIELD_NUMBER, UnknownFieldSet.Field.newBuilder() + .addLengthDelimited(ByteString.copyFrom(new byte[8192])) + .build()) + .build(); + byte[] padded = backupMessage.toBuilder().setUnknownFields(unknown).build().toByteArray(); + Assert.assertTrue(padded.length > backupMessage.toByteArray().length + 8000); + + // Parsing uses DiscardUnknownFieldsParser, so the padded message still parses and its known + // fields are intact while the unknown padding is discarded from the parsed proto. + KeepAliveMessage m = new KeepAliveMessage(padded); + Assert.assertTrue(m.getFlag()); + Assert.assertEquals(10, m.getPriority()); + } + @Test public void test() throws Exception { KeepAliveMessage m1 = new KeepAliveMessage(true, 10); diff --git a/framework/src/test/java/org/tron/core/net/MessageUnknownFieldTest.java b/framework/src/test/java/org/tron/core/net/MessageUnknownFieldTest.java new file mode 100644 index 00000000000..362d4e3d37f --- /dev/null +++ b/framework/src/test/java/org/tron/core/net/MessageUnknownFieldTest.java @@ -0,0 +1,97 @@ +package org.tron.core.net; + +import com.google.protobuf.ByteString; +import com.google.protobuf.Message; +import com.google.protobuf.UnknownFieldSet; +import org.junit.Assert; +import org.junit.Test; +import org.tron.core.net.message.adv.FetchInvDataMessage; +import org.tron.core.net.message.adv.InventoryMessage; +import org.tron.core.net.message.base.DisconnectMessage; +import org.tron.core.net.message.handshake.HelloMessage; +import org.tron.core.net.message.sync.ChainInventoryMessage; +import org.tron.protos.Protocol; +import org.tron.protos.Protocol.Inventory.InventoryType; +import org.tron.protos.Protocol.ReasonCode; + +/** + * Verifies that transient control messages (not persisted to the database) drop attacker-appended + * unknown fields at parse time via {@code DiscardUnknownFieldsParser}, so padding cannot be + * retained in memory. trx/trxs/block/pbft messages are intentionally excluded because they are + * stored and rely on their exact serialized bytes for hashing/signature verification. + */ +public class MessageUnknownFieldTest { + + // A field number not used by any message under test, so it always parses as an unknown field. + private static final int UNKNOWN_FIELD_NUMBER = 1000; + private static final int PADDING_SIZE = 8192; + + private static byte[] withUnknown(Message msg) { + UnknownFieldSet unknown = UnknownFieldSet.newBuilder() + .addField(UNKNOWN_FIELD_NUMBER, UnknownFieldSet.Field.newBuilder() + .addLengthDelimited(ByteString.copyFrom(new byte[PADDING_SIZE])) + .build()) + .build(); + byte[] padded = msg.toBuilder().setUnknownFields(unknown).build().toByteArray(); + // Sanity: the padded bytes really are larger than the clean message. + Assert.assertTrue(padded.length > msg.toByteArray().length + PADDING_SIZE - 16); + return padded; + } + + @Test + public void inventoryDiscardsUnknownFields() throws Exception { + Protocol.Inventory inv = Protocol.Inventory.newBuilder() + .setType(InventoryType.TRX) + .addIds(ByteString.copyFrom(new byte[32])) + .build(); + InventoryMessage msg = new InventoryMessage(withUnknown(inv)); + Assert.assertTrue(msg.getInventory().getUnknownFields().asMap().isEmpty()); + Assert.assertEquals(inv, msg.getInventory()); + Assert.assertEquals(1, msg.getHashList().size()); + } + + @Test + public void fetchInvDataDiscardsUnknownFields() throws Exception { + Protocol.Inventory inv = Protocol.Inventory.newBuilder() + .setType(InventoryType.BLOCK) + .addIds(ByteString.copyFrom(new byte[32])) + .build(); + FetchInvDataMessage msg = new FetchInvDataMessage(withUnknown(inv)); + Assert.assertTrue(msg.getInventory().getUnknownFields().asMap().isEmpty()); + Assert.assertEquals(inv, msg.getInventory()); + } + + @Test + public void helloMessageDiscardsUnknownFields() throws Exception { + Protocol.HelloMessage hello = Protocol.HelloMessage.newBuilder() + .setVersion(1) + .setTimestamp(123456789L) + .build(); + HelloMessage msg = new HelloMessage(withUnknown(hello)); + Assert.assertTrue(msg.getHelloMessage().getUnknownFields().asMap().isEmpty()); + Assert.assertEquals(1, msg.getVersion()); + Assert.assertEquals(123456789L, msg.getTimestamp()); + } + + @Test + public void chainInventoryDiscardsUnknownFields() throws Exception { + Protocol.ChainInventory chainInv = Protocol.ChainInventory.newBuilder() + .addIds(Protocol.ChainInventory.BlockId.newBuilder() + .setHash(ByteString.copyFrom(new byte[32])).setNumber(1).build()) + .setRemainNum(0) + .build(); + ChainInventoryMessage msg = new ChainInventoryMessage(withUnknown(chainInv)); + // Known fields survive; padding is dropped so the message re-serializes to its clean size. + Assert.assertEquals(1, msg.getBlockIds().size()); + Assert.assertEquals(Long.valueOf(0L), msg.getRemainNum()); + } + + @Test + public void disconnectMessageDiscardsUnknownFields() throws Exception { + Protocol.DisconnectMessage disconnect = Protocol.DisconnectMessage.newBuilder() + .setReason(ReasonCode.TOO_MANY_PEERS) + .build(); + DisconnectMessage msg = new DisconnectMessage(withUnknown(disconnect)); + Assert.assertEquals(ReasonCode.TOO_MANY_PEERS, msg.getReason()); + } +}