Correct the PointGFp check for CVE-2016-21951.10.12

Remi Gacogne pointed out that the check intended for the y coordinate
instead checks x again. The overflow is still avoided in 1.10.11
however, because of the size check in bigint_mul and bigint_sqr also
added in that release.

4 files changed, 61 insertions, 3 deletions

diff --git a/botan_version.py b/botan_version.py
index 7f3eccaa5..d4f7b56da 100644
--- a/botan_version.py
+++ b/botan_version.py
@@ -1,7 +1,7 @@
 
 release_major = 1
 release_minor = 10
-release_patch = 11
+release_patch = 12
 
 release_so_abi_rev = 1
 
diff --git a/checks/ec_tests.cpp b/checks/ec_tests.cpp
index 8ed975603..2ffae7de6 100644
--- a/checks/ec_tests.cpp
+++ b/checks/ec_tests.cpp
@@ -784,6 +784,49 @@ void test_curve_cp_ctor()
    CurveGFp curve(dom_pars.get_curve());
    }
 
+size_t test_cve_2016_2195()
+   {
+   EC_Group dom_pars("secp256r1");
+   CurveGFp curve(dom_pars.get_curve());
+
+   size_t fail = 0;
+
+   const BigInt p = curve.get_p();
+
+   try {
+     PointGFp point(curve, p, p - 1);
+     std::cout << "Accepted PointGFp x == p\n";
+     ++fail;
+   }
+   catch(...) {}
+
+   try {
+     PointGFp point(curve, p + 1, p - 1);
+     std::cout << "Accepted PointGFp x > p\n";
+     ++fail;
+   }
+   catch(...) {}
+
+   try {
+     PointGFp point(curve, p - 1, p);
+     std::cout << "Accepted PointGFp y == p\n";
+     ++fail;
+   }
+   catch(...) {}
+
+   try {
+     PointGFp point(curve, p - 1, p + 1);
+     std::cout << "Accepted PointGFp y > p\n";
+     ++fail;
+   }
+   catch(...) {}
+
+   // this is allowed (though not on the curve)
+   PointGFp point(curve, p - 1, p - 1);
+
+   return fail;
+   }
+
 }
 
 void do_ec_tests(RandomNumberGenerator& rng)
@@ -814,6 +857,7 @@ void do_ec_tests(RandomNumberGenerator& rng)
    test_point_swap(rng);
    test_mult_sec_mass(rng);
    test_curve_cp_ctor();
+   test_cve_2016_2195();
 
    std::cout << std::endl;
    }
diff --git a/doc/log.txt b/doc/log.txt
index 14430a3ed..dc744f59c 100644
--- a/doc/log.txt
+++ b/doc/log.txt
@@ -7,10 +7,24 @@ Release Notes
 Series 1.10
 ----------------------------------------
 
+Version 1.10.12, 2016-02-03
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+* In 1.10.11, the check in PointGFp intended to check the affine y
+  argument actually checked the affine x again. Reported by Remi Gacogne
+
+  The CVE-2016-2195 overflow is not exploitable in 1.10.11 due to an
+  additional check in the multiplication function itself which was
+  also added in that release, so there are no security implications
+  from the missed check. However to avoid confusion the change was
+  pushed in a new release immediately.
+
+  The 1.10.11 release notes incorrectly identified CVE-2016-2195 as CVE-2016-2915
+
 Version 1.10.11, 2016-02-01
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-* Resolve heap overflow in ECC point decoding. CVE-2016-2915
+* Resolve heap overflow in ECC point decoding. CVE-2016-2195
 
 * Resolve infinite loop in modular square root algorithm.
   CVE-2016-2194
diff --git a/src/math/ec_gfp/point_gfp.cpp b/src/math/ec_gfp/point_gfp.cpp
index afd3b9d32..add1e43e9 100644
--- a/src/math/ec_gfp/point_gfp.cpp
+++ b/src/math/ec_gfp/point_gfp.cpp
@@ -28,7 +28,7 @@ PointGFp::PointGFp(const CurveGFp& curve, const BigInt& x, const BigInt& y) :
    {
    if(x <= 0 || x >= curve.get_p())
       throw Invalid_Argument("Invalid PointGFp x");
-   if(x <= 0 || x >= curve.get_p())
+   if(y <= 0 || y >= curve.get_p())
       throw Invalid_Argument("Invalid PointGFp y");
    coord_x = monty_mult(x, curve.get_r2());
    coord_y = monty_mult(y, curve.get_r2());