Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.UnknownFieldSet;
import java.security.SignatureException;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -328,6 +329,26 @@ public boolean hasWitnessSignature() {
return !getInstance().getBlockHeader().getWitnessSignature().isEmpty();
}

public boolean sanitize() {
boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty();
boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty();
if (!blockHasUnknown && !headerHasUnknown) {
return false;
}
UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
Block.Builder builder = this.block.toBuilder();
if (blockHasUnknown) {
builder.setUnknownFields(empty);
}
if (headerHasUnknown) {
builder.setBlockHeader(this.block.getBlockHeader().toBuilder()
.setUnknownFields(empty)
.build());
}
this.block = builder.build();
return true;
}

@Override
public String toString() {
StringBuilder toStringBuff = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.protobuf.GeneratedMessageV3;
import com.google.protobuf.Internal;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.UnknownFieldSet;
import java.io.IOException;
import java.security.SignatureException;
import java.util.ArrayList;
Expand Down Expand Up @@ -494,6 +495,16 @@ public static boolean validateSignature(Transaction transaction,
return false;
}

public boolean sanitize() {
if (this.transaction.getUnknownFields().asMap().isEmpty()) {
return false;
}
this.transaction = this.transaction.toBuilder()
.setUnknownFields(UnknownFieldSet.getDefaultInstance())
.build();
return true;
}

public void resetResult() {
if (this.getInstance().getRetCount() > 0) {
this.transaction = this.getInstance().toBuilder().clearRet().build();
Expand Down
2 changes: 1 addition & 1 deletion framework/src/main/java/org/tron/core/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,9 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) {
if (trx.getInstance().getRawData().getContractCount() == 0) {
throw new ContractValidateException(ActuatorConstant.CONTRACT_NOT_EXIST);
}
TransactionMessage message = new TransactionMessage(trx.getInstance().toByteArray());
trx.checkExpiration(chainBaseManager.getNextBlockSlotTime());
dbManager.pushTransaction(trx);
TransactionMessage message = new TransactionMessage(trx.getInstance().toByteArray());
int num = tronNetService.fastBroadcastTransaction(message);
if (num == 0 && minEffectiveConnection != 0) {
return builder.setResult(false).setCode(response_code.NOT_ENOUGH_EFFECTIVE_CONNECTION)
Expand Down
3 changes: 3 additions & 0 deletions framework/src/main/java/org/tron/core/db/Manager.java
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,9 @@ public TransactionInfo processTransaction(final TransactionCapsule trxCap, Block
String.format(" %s transaction signature validate failed", txId));
}

if (!trxCap.isInBlock()) {
trxCap.sanitize();
}
TransactionTrace trace = new TransactionTrace(trxCap, StoreFactory.getInstance(),
new RuntimeImpl());
trxCap.setTrxTrace(trace);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ public BlockMessage(BlockCapsule block) {
this.block = block;
}

public void sanitize() {
if (this.block.sanitize()) {
this.data = this.block.getData();
}
}
Comment thread
halibobo1205 marked this conversation as resolved.

public BlockId getBlockId() {
return getBlockCapsule().getBlockId();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public void processMessage(PeerConnection peer, TronMessage msg) throws P2pExcep
check(peer, blockMessage);
}

blockMessage.sanitize();

if (peer.getSyncBlockRequested().containsKey(blockId)) {
peer.getSyncBlockRequested().remove(blockId);
peer.getSyncBlockInProcess().add(blockId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
package org.tron.core.net.message.adv;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import com.google.protobuf.ByteString;
import com.google.protobuf.UnknownFieldSet;
import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.Mockito;
import org.tron.common.overlay.message.Message;
import org.tron.core.capsule.BlockCapsule;
import org.tron.core.capsule.TransactionCapsule;
import org.tron.core.store.DynamicPropertiesStore;
import org.tron.protos.Protocol.Block;
import org.tron.protos.Protocol.BlockHeader;
import org.tron.protos.Protocol.Transaction;

/**
* Verifies the {@code sanitize()} helpers on {@link BlockCapsule},
* {@link TransactionCapsule} and {@link BlockMessage}: they strip outer
* unknown protobuf fields while leaving every consensus-hashed / signed
* region byte-identical.
*/
public class SanitizeUnknownFieldsTest {

private static final UnknownFieldSet PADDING = UnknownFieldSet.newBuilder()
.addField(99999, UnknownFieldSet.Field.newBuilder()
.addLengthDelimited(ByteString.copyFrom(new byte[1024]))
.build())
.build();

@BeforeClass
public static void setUp() {
// BlockMessage(byte[]) calls Message.isFilter() which dereferences the
// static DynamicPropertiesStore. The mock's primitive-long getter returns
// 0L by default, so isFilter() returns false.
Message.setDynamicPropertiesStore(Mockito.mock(DynamicPropertiesStore.class));
}

private static BlockHeader.raw sampleRawHeader() {
return BlockHeader.raw.newBuilder()
.setNumber(100)
.setTimestamp(123456789L)
.build();
}

private static Block sampleBlock() {
return Block.newBuilder()
.setBlockHeader(BlockHeader.newBuilder().setRawData(sampleRawHeader()).build())
.build();
}

private static Transaction sampleTransaction() {
return Transaction.newBuilder()
.setRawData(Transaction.raw.newBuilder().setTimestamp(123456789L).build())
.build();
}

// ---- BlockCapsule.sanitize ----

@Test
public void blockCapsuleSanitizeStripsBlockLevelUnknownFields() {
Block padded = sampleBlock().toBuilder().setUnknownFields(PADDING).build();
BlockCapsule capsule = new BlockCapsule(padded);
long originalSize = capsule.getData().length;

assertTrue("sanitize() should report it mutated the capsule", capsule.sanitize());

assertTrue("Block-level unknown fields should be stripped",
capsule.getInstance().getUnknownFields().asMap().isEmpty());
assertTrue("Sanitized capsule bytes should shrink",
capsule.getData().length < originalSize);
}

@Test
public void blockCapsuleSanitizeStripsBlockHeaderOuterUnknownFields() {
BlockHeader paddedHeader = BlockHeader.newBuilder()
.setRawData(sampleRawHeader())
.setUnknownFields(PADDING)
.build();
Block padded = Block.newBuilder().setBlockHeader(paddedHeader).build();
BlockCapsule capsule = new BlockCapsule(padded);
long originalSize = capsule.getData().length;

assertTrue("sanitize() should report it mutated the capsule", capsule.sanitize());

assertTrue("BlockHeader outer unknown fields should be stripped",
capsule.getInstance().getBlockHeader().getUnknownFields().asMap().isEmpty());
assertTrue(capsule.getData().length < originalSize);
}

@Test
public void blockCapsuleSanitizePreservesBlockHeaderRawData() {
Block clean = sampleBlock();
Block padded = clean.toBuilder().setUnknownFields(PADDING).build();
BlockCapsule capsule = new BlockCapsule(padded);

capsule.sanitize();

assertEquals("BlockHeader.raw_data must be byte-identical so block hash matches",
clean.getBlockHeader().getRawData(),
capsule.getInstance().getBlockHeader().getRawData());
}

@Test
public void blockCapsuleSanitizeIsNoOpOnCleanBlock() {
Block clean = sampleBlock();
BlockCapsule capsule = new BlockCapsule(clean);
Block beforeInstance = capsule.getInstance();
byte[] beforeData = capsule.getData();

assertFalse("sanitize() should report no-op on a clean block", capsule.sanitize());

assertSame("Underlying Block reference should not be rebuilt",
beforeInstance, capsule.getInstance());
assertArrayEquals("Clean block should pass through unchanged", beforeData, capsule.getData());
}

// ---- TransactionCapsule.sanitize ----

@Test
public void transactionCapsuleSanitizeStripsTopLevelUnknownFields() {
Transaction padded = sampleTransaction().toBuilder().setUnknownFields(PADDING).build();
TransactionCapsule capsule = new TransactionCapsule(padded);
long originalSize = capsule.getData().length;

assertTrue("sanitize() should report it mutated the capsule", capsule.sanitize());

assertTrue("Transaction-level unknown fields should be stripped",
capsule.getInstance().getUnknownFields().asMap().isEmpty());
assertTrue(capsule.getData().length < originalSize);
}

@Test
public void transactionCapsuleSanitizePreservesTransactionId() {
Transaction clean = sampleTransaction();
Transaction padded = clean.toBuilder().setUnknownFields(PADDING).build();
TransactionCapsule cleanCapsule = new TransactionCapsule(clean);
TransactionCapsule paddedCapsule = new TransactionCapsule(padded);

paddedCapsule.sanitize();

assertEquals("Padding outside raw_data must not change the transaction id",
cleanCapsule.getTransactionId(),
paddedCapsule.getTransactionId());
}

@Test
public void transactionCapsuleSanitizeIsNoOpOnCleanTransaction() {
Transaction clean = sampleTransaction();
TransactionCapsule capsule = new TransactionCapsule(clean);
Transaction beforeInstance = capsule.getInstance();
byte[] beforeData = capsule.getData();

assertFalse("sanitize() should report no-op on a clean transaction", capsule.sanitize());

assertSame("Underlying Transaction reference should not be rebuilt",
beforeInstance, capsule.getInstance());
assertArrayEquals(beforeData, capsule.getData());
}

// ---- BlockMessage.sanitize ----

@Test
public void blockMessageSanitizeUpdatesBothCapsuleAndWireBytes() throws Exception {
Block padded = sampleBlock().toBuilder().setUnknownFields(PADDING).build();
byte[] paddedBytes = padded.toByteArray();
BlockMessage msg = new BlockMessage(paddedBytes);
assertArrayEquals("Constructor should not sanitize on its own",
paddedBytes, msg.getData());

msg.sanitize();

assertTrue("BlockCapsule should be sanitized",
msg.getBlockCapsule().getInstance().getUnknownFields().asMap().isEmpty());
assertTrue("msg.data should also be rewritten to canonical bytes",
msg.getData().length < paddedBytes.length);
assertArrayEquals("msg.data should equal capsule.getData() after sanitize",
msg.getBlockCapsule().getData(), msg.getData());
assertNotEquals("msg.data should no longer match the padded wire bytes",
paddedBytes.length, msg.getData().length);
}

@Test
public void blockMessageSanitizeSkipsDataRewriteOnCleanBlock() throws Exception {
byte[] cleanBytes = sampleBlock().toByteArray();
BlockMessage msg = new BlockMessage(cleanBytes);
byte[] before = msg.getData();

msg.sanitize();

assertSame("msg.data should not be rewritten on the no-op path",
before, msg.getData());
}
}
Loading