fix(net): dedupe PBFT quorum by recovered SR address

Allowing 65-68 byte PBFT signatures permitted trailing-byte variants of
the same physical signature to be counted as distinct votes: the quorum
set was keyed by raw signature bytes while ECDSA recovery only consumes
the first 65 bytes, so a single signer could submit padded copies and
inflate their vote weight.

Switch validPbftSign to count unique recovered SR addresses instead, and
add a regression test exercising sigs padded to 65/66/67/68 bytes from
the same signer.

Author: Federico2014

framework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.java

@@ -124,19 +124,13 @@ private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
       List<ByteString> currentSrList) {
     //valid sr list
     if (srSignList.size() != 0) {
-      Set<ByteString> srSignSet = new ConcurrentSet();
-      srSignSet.addAll(srSignList);
-      if (srSignSet.size() < Param.getInstance().getAgreeNodeCount()) {
-        logger.error("sr sign count {} < sr count * 2/3 + 1 == {}", srSignSet.size(),
-            Param.getInstance().getAgreeNodeCount());
-        return false;
-      }
       byte[] dataHash = Sha256Hash.hash(true, raw.toByteArray());
       Set<ByteString> srSet = Sets.newHashSet(currentSrList);
+      Set<ByteString> validSrAddressSet = new ConcurrentSet();
       List<Future<Boolean>> futureList = new ArrayList<>();
       for (ByteString sign : srSignList) {
         futureList.add(executorService.submit(
-            new ValidPbftSignTask(raw.getViewN(), srSignSet, dataHash, srSet, sign)));
+            new ValidPbftSignTask(raw.getViewN(), validSrAddressSet, dataHash, srSet, sign)));
       }
       for (Future<Boolean> future : futureList) {
         try {
@@ -147,7 +141,9 @@ private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
           logger.error("", e);
         }
       }
-      if (srSignSet.size() != 0) {
+      if (validSrAddressSet.size() < Param.getInstance().getAgreeNodeCount()) {
+        logger.error("sr sign count {} < sr count * 2/3 + 1 == {}", validSrAddressSet.size(),
+            Param.getInstance().getAgreeNodeCount());
         return false;
       }
     }
@@ -157,15 +153,15 @@ private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
   private class ValidPbftSignTask implements Callable<Boolean> {
 
     private long viewN;
-    private Set<ByteString> srSignSet;
+    private Set<ByteString> validSrAddressSet;
     private byte[] dataHash;
     private Set<ByteString> srSet;
     private ByteString sign;
 
-    ValidPbftSignTask(long viewN, Set<ByteString> srSignSet,
+    ValidPbftSignTask(long viewN, Set<ByteString> validSrAddressSet,
         byte[] dataHash, Set<ByteString> srSet, ByteString sign) {
       this.viewN = viewN;
-      this.srSignSet = srSignSet;
+      this.validSrAddressSet = validSrAddressSet;
       this.dataHash = dataHash;
       this.srSet = srSet;
       this.sign = sign;
@@ -186,7 +182,7 @@ public Boolean call() throws Exception {
               ByteArray.toHexString(srAddress));
           return false;
         }
-        srSignSet.remove(sign);
+        validSrAddressSet.add(ByteString.copyFrom(srAddress));
       } catch (SignatureException e) {
         logger.error("viewN {} valid sr list sign fail!", viewN, e);
         return false;

framework/src/test/java/org/tron/core/net/messagehandler/PbftDataSyncHandlerTest.java

@@ -5,11 +5,17 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
+import org.bouncycastle.util.encoders.Hex;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
+import org.tron.common.crypto.SignInterface;
+import org.tron.common.crypto.SignUtils;
+import org.tron.common.utils.PublicMethod;
 import org.tron.common.utils.Sha256Hash;
 import org.tron.consensus.base.Param;
 import org.tron.core.ChainBaseManager;
@@ -89,4 +95,46 @@ public void testValidPbftSignPaddedSigOsakaRejected() throws Exception {
         Collections.singletonList(ByteString.copyFrom(new byte[21])));
     Assert.assertFalse(valid);
   }
+
+  @Test
+  public void testValidPbftSignTrailingBytePaddedDuplicatesRejected() throws Exception {
+    PbftDataSyncHandler pbftDataSyncHandler = new PbftDataSyncHandler();
+    DynamicPropertiesStore dynamicPropertiesStore = Mockito.mock(DynamicPropertiesStore.class);
+    ChainBaseManager chainBaseManager = Mockito.mock(ChainBaseManager.class);
+    Mockito.when(chainBaseManager.getDynamicPropertiesStore()).thenReturn(dynamicPropertiesStore);
+    Mockito.when(dynamicPropertiesStore.getAllowTvmOsaka()).thenReturn(1L);
+
+    Field field = PbftDataSyncHandler.class.getDeclaredField("chainBaseManager");
+    field.setAccessible(true);
+    field.set(pbftDataSyncHandler, chainBaseManager);
+
+    Param.getInstance().setAgreeNodeCount(2);
+    Method method = PbftDataSyncHandler.class.getDeclaredMethod("validPbftSign",
+        Protocol.PBFTMessage.Raw.class, java.util.List.class, java.util.List.class);
+    method.setAccessible(true);
+
+    Protocol.PBFTMessage.Raw raw = Protocol.PBFTMessage.Raw.newBuilder()
+        .setViewN(1)
+        .setEpoch(0)
+        .setDataType(Protocol.PBFTMessage.DataType.BLOCK)
+        .setMsgType(Protocol.PBFTMessage.MsgType.COMMIT)
+        .setData(ByteString.copyFromUtf8("block"))
+        .build();
+
+    SignInterface signer = SignUtils.fromPrivate(
+        Hex.decode(PublicMethod.getRandomPrivateKey()), true);
+    byte[] sig65 = signer.Base64toBytes(
+        signer.signHash(Sha256Hash.hash(true, raw.toByteArray())));
+    List<ByteString> paddedSigs = Arrays.asList(
+        ByteString.copyFrom(sig65),
+        ByteString.copyFrom(Arrays.copyOf(sig65, 66)),
+        ByteString.copyFrom(Arrays.copyOf(sig65, 67)),
+        ByteString.copyFrom(Arrays.copyOf(sig65, 68)));
+    List<ByteString> currentSrList = Collections.singletonList(
+        ByteString.copyFrom(signer.getAddress()));
+
+    boolean valid = (boolean) method.invoke(pbftDataSyncHandler, raw,
+        paddedSigs, currentSrList);
+    Assert.assertFalse("trailing-byte padded duplicates must not inflate quorum", valid);
+  }
 }