strengthen the private key validation

Author: Federico2014

chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java

@@ -85,11 +85,15 @@ private void validate(String privateKey) {
       privateKey = privateKey.substring(2);
     }
 
-    if (StringUtils.isBlank(privateKey) || (StringUtils.isNotBlank(privateKey)
-        && privateKey.length() != ChainConstant.PRIVATE_KEY_LENGTH)) {
+    if (StringUtils.isBlank(privateKey)
+        || privateKey.length() != ChainConstant.PRIVATE_KEY_LENGTH) {
       throw new IllegalArgumentException(
-          String.format("private key must be %d-bits hex string, actual: %d",
-              ChainConstant.PRIVATE_KEY_LENGTH, privateKey.length()));
+          String.format("private key must be %d hex string, actual: %d",
+              ChainConstant.PRIVATE_KEY_LENGTH,
+              StringUtils.isBlank(privateKey) ? 0 : privateKey.length()));
+    }
+    if (!StringUtil.isHexadecimal(privateKey)) {
+      throw new IllegalArgumentException("private key must be hex string");
     }
   }
 

common/src/main/java/org/tron/common/utils/StringUtil.java

@@ -44,4 +44,14 @@ public static String createReadableString(ByteString string) {
   public static ByteString hexString2ByteString(String hexString) {
     return ByteString.copyFrom(ByteArray.fromHexString(hexString));
   }
+
+  public static boolean isHexadecimal(String str) {
+    for (int i = 0; i < str.length(); i++) {
+      char c = str.charAt(i);
+      if (!((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'))) {
+        return false;
+      }
+    }
+    return true;
+  }
 }

framework/src/test/java/org/tron/core/config/args/LocalWitnessTest.java

@@ -74,6 +74,84 @@ public void whenSetPrefixPrivateKey() {
     Assert.assertNotNull(localWitness.getPrivateKey());
   }
 
+  @Test
+  public void testValidPrivateKey() {
+    LocalWitnesses localWitnesses = new LocalWitnesses();
+
+    try {
+      localWitnesses.addPrivateKeys(PRIVATE_KEY);
+      Assert.assertEquals(1, localWitnesses.getPrivateKeys().size());
+      Assert.assertEquals(PRIVATE_KEY, localWitnesses.getPrivateKeys().get(0));
+    } catch (Exception e) {
+      fail(e.getMessage());
+    }
+  }
+
+  @Test
+  public void testValidPrivateKeyWithPrefix() {
+    LocalWitnesses localWitnesses = new LocalWitnesses();
+
+    try {
+      localWitnesses.addPrivateKeys("0x" + PRIVATE_KEY);
+      Assert.assertEquals(1, localWitnesses.getPrivateKeys().size());
+      Assert.assertEquals("0x" + PRIVATE_KEY, localWitnesses.getPrivateKeys().get(0));
+    } catch (Exception e) {
+      fail(e.getMessage());
+    }
+  }
+
+  @Test
+  public void testInvalidPrivateKey() {
+    LocalWitnesses localWitnesses = new LocalWitnesses();
+
+    try {
+      localWitnesses.addPrivateKeys(null);
+      fail("should throw IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      Assert.assertTrue(e.getMessage().contains("private key must be"));
+    } catch (Exception e) {
+      fail("should IllegalArgumentException，actual exception: " + e.getClass().getSimpleName());
+    }
+
+    try {
+      localWitnesses.addPrivateKeys("");
+      fail("should throw IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      Assert.assertTrue(e.getMessage().contains("private key must be"));
+    } catch (Exception e) {
+      fail("should IllegalArgumentException，actual exception: " + e.getClass().getSimpleName());
+    }
+
+    try {
+      localWitnesses.addPrivateKeys("  ");
+      fail("should throw IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      Assert.assertTrue(e.getMessage().contains("private key must be"));
+    } catch (Exception e) {
+      fail("should IllegalArgumentException，actual exception: " + e.getClass().getSimpleName());
+    }
+
+    try {
+      String privateKey = "11111111111111111111111111111111111111111111111111111111111111  ";
+      localWitnesses.addPrivateKeys(privateKey);
+      fail("should throw IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      Assert.assertTrue(e.getMessage().contains("private key must be hex string"));
+    } catch (Exception e) {
+      fail("should IllegalArgumentException，actual exception: " + e.getClass().getSimpleName());
+    }
+
+    try {
+      String privateKey = "xy11111111111111111111111111111111111111111111111111111111111111";
+      localWitnesses.addPrivateKeys(privateKey);
+      fail("should throw IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      Assert.assertTrue(e.getMessage().contains("private key must be hex string"));
+    } catch (Exception e) {
+      fail("should IllegalArgumentException，actual exception: " + e.getClass().getSimpleName());
+    }
+  }
+
   @Test
   public void getPrivateKey() {
     Assert.assertEquals(Lists