Skip to content

Commit 37c7ab5

Browse files
committed
fix(crypto): address security review findings on HTTP status, null guards, and key length
1 parent a94a3ec commit 37c7ab5

4 files changed

Lines changed: 28 additions & 0 deletions

File tree

crypto/src/main/java/org/tron/common/crypto/ECKey.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,11 @@ public static boolean isValidPrivateKey(byte[] keyBytes) {
266266
if (ByteArray.isEmpty(keyBytes)) {
267267
return false;
268268
}
269+
// Accept a 33-byte array only when the leading byte is 0x00 (BigInteger sign-byte padding);
270+
// reject anything longer or any non-canonical 33-byte encoding.
271+
if (keyBytes.length > 33 || (keyBytes.length == 33 && keyBytes[0] != 0x00)) {
272+
return false;
273+
}
269274

270275
BigInteger key = new BigInteger(1, keyBytes);
271276
return key.compareTo(BigInteger.ONE) >= 0 && key.compareTo(CURVE.getN()) < 0;
@@ -350,6 +355,9 @@ public static ECKey fromPrivate(byte[] privKeyBytes) {
350355
* @return -
351356
*/
352357
public static ECKey fromPublicOnly(byte[] pub) {
358+
if (ByteArray.isEmpty(pub)) {
359+
throw new IllegalArgumentException("Public key bytes cannot be null or empty");
360+
}
353361
return new ECKey(null, CURVE.getCurve().decodePoint(pub));
354362
}
355363

@@ -417,6 +425,9 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, String signatureBas
417425

418426
public static byte[] signatureToKeyBytes(byte[] messageHash,
419427
ECDSASignature sig) throws SignatureException {
428+
if (messageHash == null || sig == null) {
429+
throw new IllegalArgumentException("messageHash and sig cannot be null");
430+
}
420431
check(messageHash.length == 32, "messageHash argument has length " +
421432
messageHash.length);
422433
int header = sig.v;

crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,11 @@ public static boolean isValidPrivateKey(byte[] keyBytes) {
272272
if (ByteArray.isEmpty(keyBytes)) {
273273
return false;
274274
}
275+
// Accept a 33-byte array only when the leading byte is 0x00 (BigInteger sign-byte padding);
276+
// reject anything longer or any non-canonical 33-byte encoding.
277+
if (keyBytes.length > 33 || (keyBytes.length == 33 && keyBytes[0] != 0x00)) {
278+
return false;
279+
}
275280

276281
BigInteger key = new BigInteger(1, keyBytes);
277282
return key.compareTo(BigInteger.ONE) >= 0 && key.compareTo(SM2_N) < 0;

framework/src/main/java/org/tron/core/services/http/ScanShieldedTRC20NotesByIvkServlet.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
5353
ivkDecryptTRC20Parameters.getAk().toByteArray(),
5454
ivkDecryptTRC20Parameters.getNk().toByteArray());
5555
response.getWriter().println(convertOutput(notes, params.isVisible()));
56+
} catch (IllegalArgumentException e) {
57+
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
58+
Util.processError(e, response);
5659
} catch (Exception e) {
5760
Util.processError(e, response);
5861
}
@@ -82,6 +85,9 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
8285
ByteArray.fromHexString(contractAddress), ByteArray.fromHexString(ivk),
8386
ByteArray.fromHexString(ak), ByteArray.fromHexString(nk));
8487
response.getWriter().println(convertOutput(notes, visible));
88+
} catch (IllegalArgumentException e) {
89+
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
90+
Util.processError(e, response);
8591
} catch (Exception e) {
8692
Util.processError(e, response);
8793
}

framework/src/main/java/org/tron/core/services/http/ScanShieldedTRC20NotesByOvkServlet.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
3535
ovkDecryptTRC20Parameters.getShieldedTRC20ContractAddress().toByteArray());
3636
response.getWriter()
3737
.println(ScanShieldedTRC20NotesByIvkServlet.convertOutput(notes, params.isVisible()));
38+
} catch (IllegalArgumentException e) {
39+
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
40+
Util.processError(e, response);
3841
} catch (Exception e) {
3942
Util.processError(e, response);
4043
}
@@ -60,6 +63,9 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
6063

6164
response.getWriter()
6265
.println(ScanShieldedTRC20NotesByIvkServlet.convertOutput(notes, visible));
66+
} catch (IllegalArgumentException e) {
67+
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
68+
Util.processError(e, response);
6369
} catch (Exception e) {
6470
Util.processError(e, response);
6571
}

0 commit comments

Comments
 (0)