Skip to content

Commit a9bdbae

Browse files
Federico2014Federico2014
committed
fix(api): bound and truncate sign api signatures
1 parent 4e80f8f commit a9bdbae

5 files changed

Lines changed: 190 additions & 17 deletions

File tree

‎actuator/src/main/java/org/tron/core/utils/TransactionUtil.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,11 @@ public static String makeUpperCamelMethod(String originName) {
186186
public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
187187
TransactionSignWeight.Builder tswBuilder = TransactionSignWeight.newBuilder();
188188
TransactionExtention.Builder trxExBuilder = TransactionExtention.newBuilder();
189-
trxExBuilder.setTransaction(trx);
190189
trxExBuilder.setTxid(ByteString.copyFrom(Sha256Hash.hash(CommonParameter
191190
.getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray())));
192191
Return.Builder retBuilder = Return.newBuilder();
193192
retBuilder.setResult(true).setCode(response_code.SUCCESS);
194193
trxExBuilder.setResult(retBuilder);
195-
tswBuilder.setTransaction(trxExBuilder);
196194
Result.Builder resultBuilder = Result.newBuilder();
197195

198196
if (trx.getRawData().getContractCount() == 0) {
@@ -249,6 +247,9 @@ public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
249247
}
250248
}
251249

250+
trx = TransactionCapsule.truncateSignatures(trx);
251+
trxExBuilder.setTransaction(trx);
252+
tswBuilder.setTransaction(trxExBuilder);
252253
tswBuilder.setResult(resultBuilder);
253254
return tswBuilder.build();
254255
}

‎chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static org.tron.common.utils.StringUtil.encode58Check;
1919
import static org.tron.common.utils.WalletUtil.checkPermissionOperations;
2020
import static org.tron.core.Constant.MAX_CONTRACT_RESULT_SIZE;
21+
import static org.tron.core.Constant.PER_SIGN_LENGTH;
2122
import static org.tron.core.exception.P2pException.TypeEnum.PROTOBUF_ERROR;
2223

2324
import com.google.common.primitives.Bytes;
@@ -251,7 +252,7 @@ public static long checkWeight(Permission permission, List<ByteString> sigs, byt
251252
long weight = getWeight(permission, address);
252253
if (weight == 0) {
253254
throw new PermissionException(
254-
ByteArray.toHexString(sig.toByteArray()) + " is signed by " + encode58Check(address)
255+
ByteArray.toHexString(hash) + " is signed by " + encode58Check(address)
255256
+ " but it is not contained of permission.");
256257
}
257258
if (ForkController.instance().pass(Parameter.ForkBlockVersionEnum.VERSION_4_7_1)) {
@@ -465,6 +466,25 @@ public static String getBase64FromByteString(ByteString sign) {
465466
return ECDSASignature.fromComponents(rsv.getR(), rsv.getS(), rsv.getV()).toBase64();
466467
}
467468

469+
public static Transaction truncateSignatures(Transaction trx) {
470+
boolean needTruncate = false;
471+
for (ByteString sig : trx.getSignatureList()) {
472+
if (sig.size() > PER_SIGN_LENGTH) {
473+
needTruncate = true;
474+
break;
475+
}
476+
}
477+
if (!needTruncate) {
478+
return trx;
479+
}
480+
Transaction.Builder builder = trx.toBuilder().clearSignature();
481+
for (ByteString sig : trx.getSignatureList()) {
482+
builder.addSignature(
483+
sig.size() > PER_SIGN_LENGTH ? sig.substring(0, PER_SIGN_LENGTH) : sig);
484+
}
485+
return builder.build();
486+
}
487+
468488
public static boolean validateSignature(Transaction transaction,
469489
byte[] hash, AccountStore accountStore, DynamicPropertiesStore dynamicPropertiesStore)
470490
throws PermissionException, SignatureException, SignatureFormatException {
@@ -631,7 +651,7 @@ public void addSign(byte[] privateKey, AccountStore accountStore)
631651
.signHash(getTransactionId().getBytes())));
632652
this.transaction = this.transaction.toBuilder().addSignature(sig).build();
633653
}
634-
654+
635655
private static void checkPermission(int permissionId, Permission permission, Transaction.Contract contract) throws PermissionException {
636656
if (permissionId != 0) {
637657
if (permission.getType() != PermissionType.Active) {
@@ -714,7 +734,7 @@ public boolean validateSignature(AccountStore accountStore,
714734
}
715735
}
716736
isVerified = true;
717-
}
737+
}
718738
return true;
719739
}
720740

‎framework/src/main/java/org/tron/core/Wallet.java‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@
228228
import org.tron.protos.Protocol.MarketOrderPairList;
229229
import org.tron.protos.Protocol.MarketPrice;
230230
import org.tron.protos.Protocol.MarketPriceList;
231+
import org.tron.protos.Protocol.Permission;
232+
import org.tron.protos.Protocol.Permission.PermissionType;
231233
import org.tron.protos.Protocol.Proposal;
232234
import org.tron.protos.Protocol.Transaction;
233235
import org.tron.protos.Protocol.Transaction.Contract;
@@ -629,13 +631,11 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) {
629631
public TransactionApprovedList getTransactionApprovedList(Transaction trx) {
630632
TransactionApprovedList.Builder tswBuilder = TransactionApprovedList.newBuilder();
631633
TransactionExtention.Builder trxExBuilder = TransactionExtention.newBuilder();
632-
trxExBuilder.setTransaction(trx);
633634
trxExBuilder.setTxid(ByteString.copyFrom(Sha256Hash.hash(CommonParameter
634635
.getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray())));
635636
Return.Builder retBuilder = Return.newBuilder();
636637
retBuilder.setResult(true).setCode(response_code.SUCCESS);
637638
trxExBuilder.setResult(retBuilder);
638-
tswBuilder.setTransaction(trxExBuilder);
639639
TransactionApprovedList.Result.Builder resultBuilder = TransactionApprovedList.Result
640640
.newBuilder();
641641

@@ -650,21 +650,26 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) {
650650
if (account == null) {
651651
throw new PermissionException("Account does not exist!");
652652
}
653+
int permissionId = contract.getPermissionId();
654+
Permission permission = account.getPermissionById(permissionId);
655+
if (permission == null) {
656+
throw new PermissionException("Permission for this, does not exist!");
657+
}
658+
if (permissionId != 0) {
659+
if (permission.getType() != PermissionType.Active) {
660+
throw new PermissionException("Permission type is wrong!");
661+
}
662+
//check operations
663+
if (!WalletUtil.checkPermissionOperations(permission, contract)) {
664+
throw new PermissionException("Permission denied!");
665+
}
666+
}
653667

654668
if (trx.getSignatureCount() > 0) {
655669
List<ByteString> approveList = new ArrayList<ByteString>();
656670
byte[] hash = Sha256Hash.hash(CommonParameter
657671
.getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray());
658-
for (ByteString sig : trx.getSignatureList()) {
659-
if (sig.size() < 65) {
660-
throw new SignatureFormatException(
661-
"Signature size is " + sig.size());
662-
}
663-
String base64 = TransactionCapsule.getBase64FromByteString(sig);
664-
byte[] address = SignUtils.signatureToAddress(hash, base64, Args.getInstance()
665-
.isECKeyCryptoEngine());
666-
approveList.add(ByteString.copyFrom(address)); //out put approve list.
667-
}
672+
TransactionCapsule.checkWeight(permission, trx.getSignatureList(), hash, approveList);
668673
tswBuilder.addAllApprovedList(approveList);
669674
}
670675
resultBuilder.setCode(TransactionApprovedList.Result.response_code.SUCCESS);
@@ -680,6 +685,9 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) {
680685
}
681686
}
682687

688+
trx = TransactionCapsule.truncateSignatures(trx);
689+
trxExBuilder.setTransaction(trx);
690+
tswBuilder.setTransaction(trxExBuilder);
683691
tswBuilder.setResult(resultBuilder);
684692
return tswBuilder.build();
685693
}

‎framework/src/test/java/org/tron/core/WalletTest.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,5 +1440,96 @@ public void testGetSolidBlock() {
14401440
Block block = wallet.getSolidBlock();
14411441
assertEquals(block2, block);
14421442
}
1443+
1444+
@Test
1445+
public void testGetTransactionApprovedListSignatureBound() {
1446+
ECKey ecKey = new ECKey(Utils.getRandom());
1447+
AccountCapsule owner = new AccountCapsule(
1448+
ByteString.copyFromUtf8("approved-owner"),
1449+
ByteString.copyFrom(ecKey.getAddress()),
1450+
Protocol.AccountType.Normal,
1451+
initBalance);
1452+
chainBaseManager.getAccountStore().put(ecKey.getAddress(), owner);
1453+
// Default owner permission: a single key with weight 1, so keysCount == 1.
1454+
int keysCount = owner.getPermissionById(0).getKeysCount();
1455+
assertEquals(1, keysCount);
1456+
1457+
Transaction unsigned = Transaction.newBuilder().setRawData(
1458+
Transaction.raw.newBuilder().addContract(
1459+
Contract.newBuilder().setType(ContractType.TransferContract)
1460+
.setParameter(Any.pack(TransferContract.newBuilder().setAmount(1)
1461+
.setOwnerAddress(ByteString.copyFrom(ecKey.getAddress()))
1462+
.setToAddress(ByteString.copyFrom(
1463+
ByteArray.fromHexString(RECEIVER_ADDRESS)))
1464+
.build())).build()).build()).build();
1465+
1466+
// One valid 65-byte [r][s][recId] signature by the owner.
1467+
TransactionCapsule capsule = new TransactionCapsule(unsigned);
1468+
capsule.sign(ecKey.getPrivKeyBytes());
1469+
ByteString oneSig = capsule.getInstance().getSignature(0);
1470+
1471+
// Within keysCount: the single valid signature is recovered, result is SUCCESS.
1472+
GrpcAPI.TransactionApprovedList okList = wallet.getTransactionApprovedList(
1473+
unsigned.toBuilder().addSignature(oneSig).build());
1474+
assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.SUCCESS,
1475+
okList.getResult().getCode());
1476+
assertEquals(1, okList.getApprovedListCount());
1477+
1478+
// More signatures than keysCount: checkWeight rejects before recovering any of them,
1479+
// so the unbounded ecrecover loop can no longer be triggered.
1480+
Transaction.Builder overLimit = unsigned.toBuilder();
1481+
for (int i = 0; i < keysCount + 1; i++) {
1482+
overLimit.addSignature(oneSig);
1483+
}
1484+
GrpcAPI.TransactionApprovedList rejected =
1485+
wallet.getTransactionApprovedList(overLimit.build());
1486+
assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.OTHER_ERROR,
1487+
rejected.getResult().getCode());
1488+
assertEquals(0, rejected.getApprovedListCount());
1489+
Assert.assertTrue(rejected.getResult().getMessage().contains("more than key counts"));
1490+
}
1491+
1492+
@Test
1493+
public void testGetTransactionApprovedListTruncatesOversizedSignature() {
1494+
ECKey ecKey = new ECKey(Utils.getRandom());
1495+
AccountCapsule owner = new AccountCapsule(
1496+
ByteString.copyFromUtf8("approved-owner-trunc"),
1497+
ByteString.copyFrom(ecKey.getAddress()),
1498+
Protocol.AccountType.Normal,
1499+
initBalance);
1500+
chainBaseManager.getAccountStore().put(ecKey.getAddress(), owner);
1501+
1502+
Transaction unsigned = Transaction.newBuilder().setRawData(
1503+
Transaction.raw.newBuilder().addContract(
1504+
Contract.newBuilder().setType(ContractType.TransferContract)
1505+
.setParameter(Any.pack(TransferContract.newBuilder().setAmount(1)
1506+
.setOwnerAddress(ByteString.copyFrom(ecKey.getAddress()))
1507+
.setToAddress(ByteString.copyFrom(
1508+
ByteArray.fromHexString(RECEIVER_ADDRESS)))
1509+
.build())).build()).build()).build();
1510+
1511+
TransactionCapsule capsule = new TransactionCapsule(unsigned);
1512+
capsule.sign(ecKey.getPrivKeyBytes());
1513+
ByteString validSig = capsule.getInstance().getSignature(0);
1514+
assertEquals(65, validSig.size());
1515+
1516+
// Pad the 65-byte signature with trailing junk bytes.
1517+
ByteString oversized = validSig.concat(
1518+
ByteString.copyFrom(new byte[] {1, 2, 3, 4, 5}));
1519+
assertEquals(70, oversized.size());
1520+
1521+
GrpcAPI.TransactionApprovedList reply = wallet.getTransactionApprovedList(
1522+
unsigned.toBuilder().addSignature(oversized).build());
1523+
1524+
// Recovery still succeeds and resolves the owner.
1525+
assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.SUCCESS,
1526+
reply.getResult().getCode());
1527+
assertEquals(1, reply.getApprovedListCount());
1528+
// The echoed-back transaction has the signature truncated to 65 bytes.
1529+
Transaction echoed = reply.getTransaction().getTransaction();
1530+
assertEquals(1, echoed.getSignatureCount());
1531+
assertEquals(65, echoed.getSignature(0).size());
1532+
assertEquals(validSig, echoed.getSignature(0));
1533+
}
14431534
}
14441535

‎framework/src/test/java/org/tron/core/actuator/utils/TransactionUtilTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,23 @@
1313
import static org.tron.core.utils.TransactionUtil.validAssetName;
1414
import static org.tron.core.utils.TransactionUtil.validTokenAbbrName;
1515

16+
import com.google.protobuf.Any;
1617
import com.google.protobuf.ByteString;
1718
import java.nio.charset.StandardCharsets;
1819
import java.util.ArrayList;
1920
import java.util.List;
21+
import javax.annotation.Resource;
2022
import lombok.extern.slf4j.Slf4j;
2123
import org.junit.Assert;
2224
import org.junit.Before;
2325
import org.junit.BeforeClass;
2426
import org.junit.Test;
27+
import org.tron.api.GrpcAPI.TransactionSignWeight;
2528
import org.tron.common.BaseTest;
2629
import org.tron.common.TestConstants;
30+
import org.tron.common.crypto.ECKey;
2731
import org.tron.common.utils.ByteArray;
32+
import org.tron.common.utils.Utils;
2833
import org.tron.core.ChainBaseManager;
2934
import org.tron.core.Constant;
3035
import org.tron.core.Wallet;
@@ -36,14 +41,19 @@
3641
import org.tron.protos.Protocol;
3742
import org.tron.protos.Protocol.AccountType;
3843
import org.tron.protos.Protocol.Transaction;
44+
import org.tron.protos.Protocol.Transaction.Contract;
3945
import org.tron.protos.Protocol.Transaction.Contract.ContractType;
4046
import org.tron.protos.contract.BalanceContract.DelegateResourceContract;
47+
import org.tron.protos.contract.BalanceContract.TransferContract;
4148

4249
@Slf4j(topic = "capsule")
4350
public class TransactionUtilTest extends BaseTest {
4451

4552
private static String OWNER_ADDRESS;
4653

54+
@Resource
55+
private TransactionUtil transactionUtil;
56+
4757
/**
4858
* Init .
4959
*/
@@ -452,4 +462,47 @@ public void testConcurrentToString() throws InterruptedException {
452462
}
453463
Assert.assertTrue(true);
454464
}
465+
466+
@Test
467+
public void testGetTransactionSignWeightTruncatesOversizedSignature() {
468+
ECKey ecKey = new ECKey(Utils.getRandom());
469+
AccountCapsule owner = new AccountCapsule(
470+
ByteString.copyFromUtf8("sign-weight-owner"),
471+
ByteString.copyFrom(ecKey.getAddress()),
472+
AccountType.Normal,
473+
10_000_000_000L);
474+
chainBaseManager.getAccountStore().put(ecKey.getAddress(), owner);
475+
476+
Transaction unsigned = Transaction.newBuilder().setRawData(
477+
Transaction.raw.newBuilder().addContract(
478+
Contract.newBuilder().setType(ContractType.TransferContract)
479+
.setParameter(Any.pack(TransferContract.newBuilder().setAmount(1)
480+
.setOwnerAddress(ByteString.copyFrom(ecKey.getAddress()))
481+
.setToAddress(ByteString.copyFrom(
482+
ByteArray.fromHexString(OWNER_ADDRESS)))
483+
.build())).build()).build()).build();
484+
485+
TransactionCapsule capsule = new TransactionCapsule(unsigned);
486+
capsule.sign(ecKey.getPrivKeyBytes());
487+
ByteString validSig = capsule.getInstance().getSignature(0);
488+
assertEquals(65, validSig.size());
489+
490+
// Pad the 65-byte signature with trailing junk bytes.
491+
ByteString oversized = validSig.concat(
492+
ByteString.copyFrom(new byte[] {1, 2, 3, 4, 5}));
493+
assertEquals(70, oversized.size());
494+
495+
TransactionSignWeight reply = transactionUtil.getTransactionSignWeight(
496+
unsigned.toBuilder().addSignature(oversized).build());
497+
498+
// Recovery still resolves the owner (weight reached the default threshold).
499+
assertEquals(TransactionSignWeight.Result.response_code.ENOUGH_PERMISSION,
500+
reply.getResult().getCode());
501+
assertEquals(1, reply.getApprovedListCount());
502+
// The echoed-back transaction has the signature truncated to 65 bytes.
503+
Transaction echoed = reply.getTransaction().getTransaction();
504+
assertEquals(1, echoed.getSignatureCount());
505+
assertEquals(65, echoed.getSignature(0).size());
506+
assertEquals(validSig, echoed.getSignature(0));
507+
}
455508
}

0 commit comments

Comments
 (0)