1 files changed, 29 insertions, 12 deletions

diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c
index 933745248d..df56484b5e 100644
--- a/crypto/ec/ec_lib.c
+++ b/crypto/ec/ec_lib.c
@@ -70,6 +70,10 @@
 
 const char EC_version[] = "EC" OPENSSL_VERSION_PTEXT;
 
+/* local function prototypes */
+
+static int ec_precompute_mont_data(EC_GROUP *group);
+
 /* functions for EC_GROUP objects */
 
 EC_GROUP *EC_GROUP_new(const EC_METHOD *meth)
@@ -318,17 +322,25 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
     } else
         BN_zero(&group->cofactor);
 
-    /*
-     * Some groups have an order with
-     * factors of two, which makes the Montgomery setup fail.
-     * |group->mont_data| will be NULL in this case.
+    /*-
+     * Access to the `mont_data` field of an EC_GROUP struct should always be
+     * guarded by an EC_GROUP_VERSION(group) check to avoid OOB accesses, as the
+     * group might come from the FIPS module, which does not define the
+     * `mont_data` field inside the EC_GROUP structure.
      */
-    if (BN_is_odd(&group->order)) {
-        return ec_precompute_mont_data(group);
+    if (EC_GROUP_VERSION(group)) {
+        /*-
+         * Some groups have an order with
+         * factors of two, which makes the Montgomery setup fail.
+         * |group->mont_data| will be NULL in this case.
+         */
+        if (BN_is_odd(&group->order))
+            return ec_precompute_mont_data(group);
+
+        BN_MONT_CTX_free(group->mont_data);
+        group->mont_data = NULL;
     }
 
-    BN_MONT_CTX_free(group->mont_data);
-    group->mont_data = NULL;
     return 1;
 }
 
@@ -1098,18 +1110,23 @@ int EC_GROUP_have_precompute_mult(const EC_GROUP *group)
                                  * been performed */
 }
 
-/*
+/*-
  * ec_precompute_mont_data sets |group->mont_data| from |group->order| and
  * returns one on success. On error it returns zero.
+ *
+ * Note: this function must be called only after verifying that
+ * EC_GROUP_VERSION(group) returns true.
+ * The reason for this is that access to the `mont_data` field of an EC_GROUP
+ * struct should always be guarded by an EC_GROUP_VERSION(group) check to avoid
+ * OOB accesses, as the group might come from the FIPS module, which does not
+ * define the `mont_data` field inside the EC_GROUP structure.
  */
+static
 int ec_precompute_mont_data(EC_GROUP *group)
 {
     BN_CTX *ctx = BN_CTX_new();
     int ret = 0;
 
-    if (!EC_GROUP_VERSION(group))
-        goto err;
-
     if (group->mont_data) {
         BN_MONT_CTX_free(group->mont_data);
         group->mont_data = NULL;