Skip to content

Commit 7ef2ffd

Browse files
committed
feat(config): change allowShieldedTransactionApi default to false for security
1. change shielded transaction API default from true to false in config.conf 2. update Args.java and CommonParameter.java to reflect new default 3. add allowShieldedTransactionApi to whitelist config 4. replace arithmetic operators with StrictMathWrapper for overflow protection 5. update test to handle ArithmeticException from StrictMathWrapper.subtractExact Closes #6611
1 parent bb8b4be commit 7ef2ffd

11 files changed

Lines changed: 26 additions & 16 deletions

File tree

common/src/main/java/org/tron/common/parameter/CommonParameter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ public class CommonParameter {
335335

336336
@Getter
337337
@Setter
338-
public boolean allowShieldedTransactionApi; // clearParam: true
338+
public boolean allowShieldedTransactionApi = false;
339339
@Getter
340340
@Setter
341341
public long blockNumForEnergyLimit;

framework/src/main/java/org/tron/core/config/args/Args.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ public static void applyConfigParams(
726726
logger.warn("Configuring [node.fullNodeAllowShieldedTransaction] will be deprecated. "
727727
+ "Please use [node.allowShieldedTransactionApi] instead.");
728728
} else {
729-
PARAMETER.allowShieldedTransactionApi = true;
729+
PARAMETER.allowShieldedTransactionApi = false;
730730
}
731731

732732
PARAMETER.zenTokenId = config.hasPath(ConfigKey.NODE_ZEN_TOKENID)

framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.tron.api.GrpcAPI;
1515
import org.tron.api.GrpcAPI.BytesMessage;
1616
import org.tron.api.GrpcAPI.ShieldedTRC20Parameters;
17+
import org.tron.common.math.StrictMathWrapper;
1718
import org.tron.common.utils.ByteArray;
1819
import org.tron.common.utils.ByteUtil;
1920
import org.tron.common.utils.Sha256Hash;
@@ -548,7 +549,7 @@ public void addSpend(
548549
byte[] path,
549550
long position) throws ZksnarkException {
550551
spends.add(new SpendDescriptionInfo(expsk, note, anchor, path, position));
551-
valueBalance += note.getValue();
552+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
552553
}
553554

554555
public void addSpend(
@@ -559,7 +560,7 @@ public void addSpend(
559560
byte[] path,
560561
long position) {
561562
spends.add(new SpendDescriptionInfo(expsk, note, alpha, anchor, path, position));
562-
valueBalance += note.getValue();
563+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
563564
}
564565

565566
public void addSpend(
@@ -571,22 +572,22 @@ public void addSpend(
571572
byte[] path,
572573
long position) {
573574
spends.add(new SpendDescriptionInfo(ak, nsk, note, alpha, anchor, path, position));
574-
valueBalance += note.getValue();
575+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
575576
}
576577

577578
public void addOutput(byte[] ovk, PaymentAddress to, long value, byte[] memo)
578579
throws ZksnarkException {
579580
Note note = new Note(to, value);
580581
note.setMemo(memo);
581582
receives.add(new ReceiveDescriptionInfo(ovk, note));
582-
valueBalance -= value;
583+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
583584
}
584585

585586
public void addOutput(byte[] ovk, DiversifierT d, byte[] pkD, long value, byte[] r, byte[] memo) {
586587
Note note = new Note(d, pkD, value, r);
587588
note.setMemo(memo);
588589
receives.add(new ReceiveDescriptionInfo(ovk, note));
589-
valueBalance -= value;
590+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
590591
}
591592

592593
public static class SpendDescriptionInfo {

framework/src/main/java/org/tron/core/zen/ZenTransactionBuilder.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import lombok.Setter;
1111
import lombok.extern.slf4j.Slf4j;
1212
import org.apache.commons.lang3.ArrayUtils;
13+
import org.tron.common.math.StrictMathWrapper;
1314
import org.tron.common.utils.ByteArray;
1415
import org.tron.common.zksnark.IncrementalMerkleVoucherContainer;
1516
import org.tron.common.zksnark.JLibrustzcash;
@@ -67,7 +68,7 @@ public ZenTransactionBuilder() {
6768

6869
public void addSpend(SpendDescriptionInfo spendDescriptionInfo) {
6970
spends.add(spendDescriptionInfo);
70-
valueBalance += spendDescriptionInfo.note.getValue();
71+
valueBalance = StrictMathWrapper.addExact(valueBalance, spendDescriptionInfo.note.getValue());
7172
}
7273

7374
public void addSpend(
@@ -76,7 +77,7 @@ public void addSpend(
7677
byte[] anchor,
7778
IncrementalMerkleVoucherContainer voucher) throws ZksnarkException {
7879
spends.add(new SpendDescriptionInfo(expsk, note, anchor, voucher));
79-
valueBalance += note.getValue();
80+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
8081
}
8182

8283
public void addSpend(
@@ -86,7 +87,7 @@ public void addSpend(
8687
byte[] anchor,
8788
IncrementalMerkleVoucherContainer voucher) {
8889
spends.add(new SpendDescriptionInfo(expsk, note, alpha, anchor, voucher));
89-
valueBalance += note.getValue();
90+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
9091
}
9192

9293
public void addSpend(
@@ -98,22 +99,22 @@ public void addSpend(
9899
byte[] anchor,
99100
IncrementalMerkleVoucherContainer voucher) {
100101
spends.add(new SpendDescriptionInfo(ak, nsk, ovk, note, alpha, anchor, voucher));
101-
valueBalance += note.getValue();
102+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
102103
}
103104

104105
public void addOutput(byte[] ovk, PaymentAddress to, long value, byte[] memo)
105106
throws ZksnarkException {
106107
Note note = new Note(to, value);
107108
note.setMemo(memo);
108109
receives.add(new ReceiveDescriptionInfo(ovk, note));
109-
valueBalance -= value;
110+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
110111
}
111112

112113
public void addOutput(byte[] ovk, DiversifierT d, byte[] pkD, long value, byte[] r, byte[] memo) {
113114
Note note = new Note(d, pkD, value, r);
114115
note.setMemo(memo);
115116
receives.add(new ReceiveDescriptionInfo(ovk, note));
116-
valueBalance -= value;
117+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
117118
}
118119

119120
public void setTransparentInput(byte[] address, long value) {

framework/src/main/resources/config.conf

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,10 @@ node {
178178

179179
minParticipationRate = 15
180180

181-
# allowShieldedTransactionApi = true
181+
# WARNING: Some shielded transaction APIs require sending private keys as parameters.
182+
# Calling these APIs on untrusted or remote nodes may leak your private keys.
183+
# It is recommended to invoke them locally for development and testing.
184+
# allowShieldedTransactionApi = false
182185

183186
# openPrintLog = true
184187

framework/src/test/java/org/tron/core/ShieldedTRC20BuilderTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public class ShieldedTRC20BuilderTest extends BaseTest {
7676
@BeforeClass
7777
public static void initZksnarkParams() {
7878
ZksnarkInitService.librustzcashInitZksnarkParams();
79+
Args.getInstance().allowShieldedTransactionApi = true;
7980
}
8081

8182
@Ignore
@@ -2461,6 +2462,4 @@ private byte[] longTo32Bytes(long value) {
24612462
byte[] zeroBytes = new byte[24];
24622463
return ByteUtil.merge(zeroBytes, longBytes);
24632464
}
2464-
2465-
24662465
}

framework/src/test/java/org/tron/core/actuator/ShieldedTransferActuatorTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,6 +1157,9 @@ public void publicAddressToShieldNoteValueFailure() {
11571157
actuator.validate();
11581158
actuator.execute(ret);
11591159
Assert.assertTrue(false);
1160+
} catch (ArithmeticException e) {
1161+
// StrictMathWrapper.subtractExact throws ArithmeticException on overflow
1162+
Assert.assertTrue(true);
11601163
} catch (ContractValidateException e) {
11611164
Assert.assertTrue(e instanceof ContractValidateException);
11621165
Assert.assertEquals("librustzcashSaplingFinalCheck error", e.getMessage());

framework/src/test/java/org/tron/core/services/RpcApiServicesTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ public class RpcApiServicesTest {
150150
public static void init() throws IOException {
151151
Args.setParam(new String[] {"-d", temporaryFolder.newFolder().toString()},
152152
TestConstants.TEST_CONF);
153+
getInstance().allowShieldedTransactionApi = true;
153154
Assert.assertEquals(5, getInstance().getRpcMaxRstStream());
154155
Assert.assertEquals(10, getInstance().getRpcSecondsPerWindow());
155156
String OWNER_ADDRESS = Wallet.getAddressPreFixString()

framework/src/test/java/org/tron/core/services/http/CreateSpendAuthSigServletTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public class CreateSpendAuthSigServletTest extends BaseTest {
2424
"--output-directory", dbPath(),
2525
}, TestConstants.TEST_CONF
2626
);
27+
Args.getInstance().allowShieldedTransactionApi = true;
2728
}
2829

2930
@Resource

framework/src/test/java/org/tron/core/zksnark/MerkleContainerTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class MerkleContainerTest extends BaseTest {
4040

4141
static {
4242
Args.setParam(new String[]{"-d", dbPath()}, TestConstants.TEST_CONF);
43+
Args.getInstance().allowShieldedTransactionApi = true;
4344
}
4445

4546
/*@Before

0 commit comments

Comments
 (0)