CVE-2020-12607: Improper Verification of Cryptographic Signature (CWE-347)
value: An issue was discovered in fastecdsa before 2.1.2. When using the NIST P-256 curve in the ECDSA implementation, the point at infinity is mishandled. This means that for an extreme value in k and s^-1, the signature verification fails even if the signature is correct. This behavior is not solely a usability problem. There are some threat models where an attacker can benefit by successfully guessing users for whom signature verification will fail.
CWE description: The product does not verify, or incorrectly verifies, the cryptographic signature for data.
CVSS v3 base score: 7.5
Published: 2020-06-02T21:15Z
Repository: https://github.com/AntonKueltz/fastecdsa
Language: C
Files changed: src/curveMath.c
Commit date: 2020-04-14 02:15:41 -0700
Commit message: Properly handle the point at infinity

Diff: diff --git a/src/curveMath.c b/src/curveMath.c
index 10160a7..0f354c6 100644
--- a/src/curveMath.c
+++ b/src/curveMath.c
@@ -13,11 +13,26 @@ int pointZZ_pEqual(const PointZZ_p * op1, const PointZZ_p * op2) {
 
     return 1;
 }
 
 
+int pointZZ_pIsIdentityElement(const PointZZ_p * op) {
+    return mpz_cmp_ui(op->x, 0) == 0 && mpz_cmp_ui(op->y, 0) == 0 ? 1 : 0;
+}
+
+
+void pointZZ_pSetToIdentityElement(PointZZ_p * op) {
+    mpz_set_ui(op->x, 0);
+    mpz_set_ui(op->y, 0);
+}
+
+
 void pointZZ_pDouble(PointZZ_p * rop, const PointZZ_p * op, const CurveZZ_p * curve) {
+    if(pointZZ_pIsIdentityElement(op)) {
+        return pointZZ_pSetToIdentityElement(rop);
+    }
+
     mpz_t numer, denom, lambda;
     mpz_inits(numer, denom, lambda, NULL);
 
     // calculate lambda
     mpz_mul(numer, op->x, op->x);
@@ -43,10 +58,39 @@ void pointZZ_pDouble(PointZZ_p * rop, const PointZZ_p * op, const CurveZZ_p * cu
     mpz_clears(numer, denom, lambda, NULL);
 }
 
 
 void pointZZ_pAdd(PointZZ_p * rop, const PointZZ_p * op1, const PointZZ_p * op2, const CurveZZ_p * curve) {
+    // handle identity element cases
+    if(pointZZ_pIsIdentityElement(op1) && pointZZ_pIsIdentityElement(op2)) {
+        return pointZZ_pSetToIdentityElement(rop);
+    } else if(pointZZ_pIsIdentityElement(op1)) {
+        mpz_set(rop->x, op2->x);
+        mpz_set(rop->y, op2->y);
+        return;
+    } else if(pointZZ_pIsIdentityElement(op2)) {
+        mpz_set(rop->x, op1->x);
+        mpz_set(rop->y, op1->y);
+        return;
+    }
+
+    // use doubling algorithm if points are equal
+    if(pointZZ_pEqual(op1, op2)) {
+        pointZZ_pDouble(rop, op1, curve);
+        return;
+    }
+
+    // check if points sum to identity element
+    mpz_t negy;
+    mpz_init(negy);
+    mpz_sub(negy, curve->p, op2->y);
+    if(mpz_cmp(op1->x, op2->x) == 0 && mpz_cmp(op1->y, negy) == 0) {
+        mpz_clear(negy);
+        return pointZZ_pSetToIdentityElement(rop);
+    }
+
+
     mpz_t xdiff, ydiff, lambda;
     mpz_inits(xdiff, ydiff, lambda, NULL);
 
     // calculate lambda
     mpz_sub(ydiff, op2->y, op1->y);
@@ -65,15 +109,20 @@ void pointZZ_pAdd(PointZZ_p * rop, const PointZZ_p * op1, const PointZZ_p * op2,
     mpz_sub(rop->y, op1->x, rop->x);
     mpz_mul(rop->y, lambda, rop->y);
     mpz_sub(rop->y, rop->y, op1->y);
     mpz_mod(rop->y, rop->y, curve->p);
 
-    mpz_clears(xdiff, ydiff, lambda, NULL);
+    mpz_clears(negy, xdiff, ydiff, lambda, NULL);
 }
 
 
 void pointZZ_pMul(PointZZ_p * rop, const PointZZ_p * point, const mpz_t scalar, const CurveZZ_p * curve) {
+    // handle the identity element
+    if(pointZZ_pIsIdentityElement(point)) {
+        return pointZZ_pSetToIdentityElement(rop);
+    }
+
     PointZZ_p R0, R1, tmp;
     mpz_inits(R1.x, R1.y, tmp.x, tmp.y, NULL);
     mpz_init_set(R0.x, point->x);
     mpz_init_set(R0.y, point->y);
     pointZZ_pDouble(&R1, point, curve);

Vulnerable code:     mpz_clears(xdiff, ydiff, lambda, NULL);
Fixed code: int pointZZ_pIsIdentityElement(const PointZZ_p * op) {
    return mpz_cmp_ui(op->x, 0) == 0 && mpz_cmp_ui(op->y, 0) == 0 ? 1 : 0;
}


void pointZZ_pSetToIdentityElement(PointZZ_p * op) {
    mpz_set_ui(op->x, 0);
    mpz_set_ui(op->y, 0);
}


    if(pointZZ_pIsIdentityElement(op)) {
        return pointZZ_pSetToIdentityElement(rop);
    }

    // handle identity element cases
    if(pointZZ_pIsIdentityElement(op1) && pointZZ_pIsIdentityElement(op2)) {
        return pointZZ_pSetToIdentityElement(rop);
    } else if(pointZZ_pIsIdentityElement(op1)) {
        mpz_set(rop->x, op2->x);
        mpz_set(rop->y, op2->y);
        return;
    } else if(pointZZ_pIsIdentityElement(op2)) {
        mpz_set(rop->x, op1->x);
        mpz_set(rop->y, op1->y);
        return;
    }

    // use doubling algorithm if points are equal
    if(pointZZ_pEqual(op1, op2)) {
        pointZZ_pDouble(rop, op1, curve);
        return;
    }

    // check if points sum to identity element
    mpz_t negy;
    mpz_init(negy);
    mpz_sub(negy, curve->p, op2->y);
    if(mpz_cmp(op1->x, op2->x) == 0 && mpz_cmp(op1->y, negy) == 0) {
        mpz_clear(negy);
        return pointZZ_pSetToIdentityElement(rop);
    }


    mpz_clears(negy, xdiff, ydiff, lambda, NULL);
    // handle the identity element
    if(pointZZ_pIsIdentityElement(point)) {
        return pointZZ_pSetToIdentityElement(rop);
    }