diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c index b55beb6c..616df162 100644 --- a/crypto/fipsmodule/ec/ec.c +++ b/crypto/fipsmodule/ec/ec.c @@ -766,6 +766,9 @@ int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group, EC_POINT *point, } if (!EC_POINT_is_on_curve(group, point, ctx)) { + // In the event of an error, defend against the caller not checking the + // return value by setting a known safe value: the base point. + EC_POINT_copy(point, EC_GROUP_get0_generator(group)); OPENSSL_PUT_ERROR(EC, EC_R_POINT_IS_NOT_ON_CURVE); return 0; } diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index 85dc8f28..25c77836 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc @@ -599,6 +599,41 @@ TEST_P(ECCurveTest, SetInvalidPrivateKey) { ERR_clear_error(); } +TEST_P(ECCurveTest, IgnoreOct2PointReturnValue) { + bssl::UniquePtr group(EC_GROUP_new_by_curve_name(GetParam().nid)); + ASSERT_TRUE(group); + + bssl::UniquePtr forty_two(BN_new()); + ASSERT_TRUE(forty_two); + ASSERT_TRUE(BN_set_word(forty_two.get(), 42)); + + // Compute g × 42. + bssl::UniquePtr point(EC_POINT_new(group.get())); + ASSERT_TRUE(point); + ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), forty_two.get(), nullptr, + nullptr, nullptr)); + + // Serialize the point. + size_t serialized_len = + EC_POINT_point2oct(group.get(), point.get(), + POINT_CONVERSION_UNCOMPRESSED, nullptr, 0, nullptr); + std::vector serialized(serialized_len); + ASSERT_EQ(serialized_len, + EC_POINT_point2oct(group.get(), point.get(), + POINT_CONVERSION_UNCOMPRESSED, serialized.data(), + serialized_len, nullptr)); + + // Create a serialized point that is not on the curve. + serialized[serialized_len - 1]++; + + ASSERT_FALSE(EC_POINT_oct2point(group.get(), point.get(), serialized.data(), + serialized.size(), nullptr)); + // After a failure, |point| should have been set to the generator to defend + // against code that doesn't check the return value. + ASSERT_EQ(0, EC_POINT_cmp(group.get(), point.get(), + EC_GROUP_get0_generator(group.get()), nullptr)); +} + static std::vector AllCurves() { const size_t num_curves = EC_get_builtin_curves(nullptr, 0); std::vector curves(num_curves);