Better error handling for invalid public keys and signatures (dashpay#41

)

* Throw error when ep_upk/ep2_upk fail

* Reset error code before throwing C++ exception

Otherwise it will keep throwing on the next CheckRelicErrors call.

* Call CheckRelicErrors after trying to read public keys or signatures

Before this, invalid public keys and signatures were silently ignored.

* Add test cases for error handling

Also move "Should throw on a bad private key" into new test case

contrib/relic/src/ep/relic_ep_util.c

@@ -294,7 +294,9 @@ void ep_read_bin(ep_t a, const uint8_t *bin, int len) {
 				THROW(ERR_NO_VALID);
 				break;
 		}
-		ep_upk(a, a);
+		if (!ep_upk(a, a)) {
+			THROW(ERR_NO_VALID);
+		}
 	}
 
 	if (len == 2 * FP_BYTES + 1) {

contrib/relic/src/epx/relic_ep2_util.c

@@ -278,7 +278,9 @@ void ep2_read_bin(ep2_t a, uint8_t *bin, int len) {
 				THROW(ERR_NO_VALID);
 				break;
 		}
-		ep2_upk(a, a);
+		if (!ep2_upk(a, a)) {
+			THROW(ERR_NO_VALID);
+		}
 	}
 
 	if (len == 4 * FP_BYTES + 1) {

src/bls.cpp

@@ -105,6 +105,7 @@ void BLS::CheckRelicErrors() {
         throw std::string("Library not initialized properly. Call BLS::Init()");
     }
     if (core_get()->code != STS_OK) {
+        core_get()->code = STS_OK;
         throw std::string("Relic library error");
     }
 }

src/publickey.cpp

@@ -33,6 +33,7 @@ PublicKey PublicKey::FromBytes(const uint8_t * key) {
         uncompressed[0] = 0x02;   // Insert extra byte for Y=0
     }
     g1_read_bin(pk.q, uncompressed, PUBLIC_KEY_SIZE + 1);
+    BLS::CheckRelicErrors();
     return pk;
 }
 

src/signature.cpp

@@ -34,6 +34,7 @@ InsecureSignature InsecureSignature::FromBytes(const uint8_t *data) {
         uncompressed[0] = 0x02;   // Insert extra byte for Y=0
     }
     g2_read_bin(sigObj.sig, uncompressed, SIGNATURE_SIZE + 1);
+    BLS::CheckRelicErrors();
     return sigObj;
 }
 

src/test.cpp

@@ -193,6 +193,51 @@ TEST_CASE("Key generation") {
     }
 }
 
+TEST_CASE("Error handling") {
+    SECTION("Should throw on a bad private key") {
+        uint8_t seed[32];
+        getRandomSeed(seed);
+        PrivateKey sk1 = PrivateKey::FromSeed(seed, 32);
+        uint8_t* skData = Util::SecAlloc<uint8_t>(
+                Signature::SIGNATURE_SIZE);
+        sk1.Serialize(skData);
+        skData[0] = 255;
+        REQUIRE_THROWS(PrivateKey::FromBytes(skData));
+
+        Util::SecFree(skData);
+    }
+
+    SECTION("Should throw on a bad public key") {
+        uint8_t buf[PublicKey::PUBLIC_KEY_SIZE] = {0};
+        std::set<int> invalid = {1, 2, 3, 4};
+
+        for (int i = 0; i < 10; i++) {
+            buf[0] = (uint8_t)i;
+            try {
+                PublicKey::FromBytes(buf);
+                REQUIRE(invalid.count(i) == 0);
+            } catch (std::string& s) {
+                REQUIRE(invalid.count(i) != 0);
+            }
+        }
+    }
+
+    SECTION("Should throw on a bad signature") {
+        uint8_t buf[Signature::SIGNATURE_SIZE] = {0};
+        std::set<int> invalid = {0, 1, 2, 3, 5, 6, 7, 8};
+
+        for (int i = 0; i < 10; i++) {
+            buf[0] = (uint8_t)i;
+            try {
+                Signature::FromBytes(buf);
+                REQUIRE(invalid.count(i) == 0);
+            } catch (std::string& s) {
+                REQUIRE(invalid.count(i) != 0);
+            }
+        }
+    }
+}
+
 TEST_CASE("Util tests") {
     SECTION("Should convert an int to four bytes") {
         uint32_t x = 1024;
@@ -335,19 +380,6 @@ TEST_CASE("Signatures") {
         REQUIRE(Signature::FromInsecureSig(sig3) == sig2);
     }
 
-    SECTION("Should throw on a bad private key") {
-        uint8_t seed[32];
-        getRandomSeed(seed);
-        PrivateKey sk1 = PrivateKey::FromSeed(seed, 32);
-        uint8_t* skData = Util::SecAlloc<uint8_t>(
-                Signature::SIGNATURE_SIZE);
-        sk1.Serialize(skData);
-        skData[0] = 255;
-        REQUIRE_THROWS(PrivateKey::FromBytes(skData));
-
-        Util::SecFree(skData);
-    }
-
     SECTION("Should not validate a bad sig") {
         uint8_t message1[7] = {100, 2, 254, 88, 90, 45, 22};
         uint8_t seed[32];