Commit e22d2199 authored by Emilia Kasper's avatar Emilia Kasper
Browse files

Error checking and memory leak fixes in NISTZ256. 



Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
parent 2f5997b7

crypto/bn/bn_err.c

+2 −1
Original line number Diff line number Diff line 
/* crypto/bn/bn_err.c */

/* ====================================================================

 * Copyright (c) 1999-2014 The OpenSSL Project.  All rights reserved.

 * Copyright (c) 1999-2015 The OpenSSL Project.  All rights reserved.

 *

 * Redistribution and use in source and binary forms, with or without

 * modification, are permitted provided that the following conditions
@@ -113,6 +113,7 @@ static ERR_STRING_DATA BN_str_functs[] = { 
    {ERR_FUNC(BN_F_BN_NEW), "BN_new"},

    {ERR_FUNC(BN_F_BN_RAND), "BN_rand"},

    {ERR_FUNC(BN_F_BN_RAND_RANGE), "BN_rand_range"},

    {ERR_FUNC(BN_F_BN_SET_WORDS), "bn_set_words"},

    {ERR_FUNC(BN_F_BN_USUB), "BN_usub"},

    {0, NULL}

};

crypto/bn/bn_intern.c

+11 −2
Original line number Diff line number Diff line
@@ -233,11 +233,20 @@ void bn_set_static_words(BIGNUM *a, BN_ULONG *words, int size) 
    a->dmax = a->top = size;

    a->neg = 0;

    a->flags |= BN_FLG_STATIC_DATA;

    bn_correct_top(a);

}



void bn_set_data(BIGNUM *a, const void *data, size_t size)

int bn_set_words(BIGNUM *a, BN_ULONG *words, int num_words)

{

    memcpy(a->d, data, size);

    if (bn_wexpand(a, num_words) == NULL) {

        BNerr(BN_F_BN_SET_WORDS, ERR_R_MALLOC_FAILURE);

        return 0;

    }



    memcpy(a->d, words, sizeof(BN_ULONG) * num_words);

    a->top = num_words;

    bn_correct_top(a);

    return 1;

}



size_t bn_sizeof_BIGNUM(void)

crypto/ec/ecp_nistz256.c

+30 −25
Original line number Diff line number Diff line
@@ -1133,6 +1133,7 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, 
    const PRECOMP256_ROW *preComputedTable = NULL;

    const EC_PRE_COMP *pre_comp = NULL;

    const EC_POINT *generator = NULL;

    BN_CTX *new_ctx = NULL;

    unsigned int idx = 0;

    const unsigned int window_size = 7;

    const unsigned int mask = (1 << (window_size + 1)) - 1;
@@ -1152,6 +1153,7 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, 
        ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, EC_R_INCOMPATIBLE_OBJECTS);

        return 0;

    }



    if ((scalar == NULL) && (num == 0))

        return EC_POINT_set_to_infinity(group, r);
@@ -1162,13 +1164,13 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, 
        }

    }



    /* Need 256 bits for space for all coordinates. */

    bn_wexpand(r->X, P256_LIMBS);

    bn_wexpand(r->Y, P256_LIMBS);

    bn_wexpand(r->Z, P256_LIMBS);

    bn_set_top(r->X, P256_LIMBS);

    bn_set_top(r->Y, P256_LIMBS);

    bn_set_top(r->Z, P256_LIMBS);

    if (ctx == NULL) {

        ctx = new_ctx = BN_CTX_new();

        if (ctx == NULL)

            goto err;

    }



    BN_CTX_start(ctx);



    if (scalar) {

        generator = EC_GROUP_get0_generator(group);
@@ -1194,8 +1196,10 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, 


            if (!ecp_nistz256_set_from_affine(pre_comp_generator,

                                              group, pre_comp->precomp[0],

                                              ctx))

                                              ctx)) {

                EC_POINT_free(pre_comp_generator);

                goto err;

            }



            if (0 == EC_POINT_cmp(group, generator, pre_comp_generator, ctx))

                preComputedTable = (const PRECOMP256_ROW *)pre_comp->precomp;
@@ -1300,14 +1304,14 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, 
        new_scalars = OPENSSL_malloc((num + 1) * sizeof(BIGNUM *));

        if (!new_scalars) {

            ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, ERR_R_MALLOC_FAILURE);

            return 0;

            goto err;

        }



        new_points = OPENSSL_malloc((num + 1) * sizeof(EC_POINT *));

        if (!new_points) {

            OPENSSL_free(new_scalars);

            ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, ERR_R_MALLOC_FAILURE);

            return 0;

            goto err;

        }



        memcpy(new_scalars, scalars, num * sizeof(BIGNUM *));
@@ -1336,18 +1340,20 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, 
        OPENSSL_free(scalars);

    }



    bn_set_data(r->X, p.p.X, sizeof(p.p.X));

    bn_set_data(r->Y, p.p.Y, sizeof(p.p.Y));

    bn_set_data(r->Z, p.p.Z, sizeof(p.p.Z));

    /* Not constant-time, but we're only operating on the public output. */

    bn_correct_top(r->X);

    bn_correct_top(r->Y);

    bn_correct_top(r->Z);

    if (!bn_set_words(r->X, p.p.X, P256_LIMBS) ||

        !bn_set_words(r->Y, p.p.Y, P256_LIMBS) ||

        !bn_set_words(r->Z, p.p.Z, P256_LIMBS)) {

        goto err;

    }

    r->Z_is_one = is_one(p.p.Z);



    ret = 1;



err:

    if (ctx)

        BN_CTX_end(ctx);

    BN_CTX_free(new_ctx);

    return ret;

}
@@ -1360,6 +1366,7 @@ static int ecp_nistz256_get_affine(const EC_GROUP *group, 
    BN_ULONG x_aff[P256_LIMBS];

    BN_ULONG y_aff[P256_LIMBS];

    BN_ULONG point_x[P256_LIMBS], point_y[P256_LIMBS], point_z[P256_LIMBS];

    BN_ULONG x_ret[P256_LIMBS], y_ret[P256_LIMBS];



    if (EC_POINT_is_at_infinity(group, point)) {

        ECerr(EC_F_ECP_NISTZ256_GET_AFFINE, EC_R_POINT_AT_INFINITY);
@@ -1378,19 +1385,17 @@ static int ecp_nistz256_get_affine(const EC_GROUP *group, 
    ecp_nistz256_mul_mont(x_aff, z_inv2, point_x);



    if (x != NULL) {

        bn_wexpand(x, P256_LIMBS);

        bn_set_top(x, P256_LIMBS);

        ecp_nistz256_from_mont(bn_get_words(x), x_aff);

        bn_correct_top(x);

        ecp_nistz256_from_mont(x_ret, x_aff);

        if (!bn_set_words(x, x_ret, P256_LIMBS))

            return 0;

    }



    if (y != NULL) {

        ecp_nistz256_mul_mont(z_inv3, z_inv3, z_inv2);

        ecp_nistz256_mul_mont(y_aff, z_inv3, point_y);

        bn_wexpand(y, P256_LIMBS);

        bn_set_top(y, P256_LIMBS);

        ecp_nistz256_from_mont(bn_get_words(y), y_aff);

        bn_correct_top(y);

        ecp_nistz256_from_mont(y_ret, y_aff);

        if (!bn_set_words(y, y_ret, P256_LIMBS))

            return 0;

    }



    return 1;

crypto/include/internal/bn_int.h

+8 −3
Original line number Diff line number Diff line
@@ -102,10 +102,15 @@ BN_ULONG *bn_get_words(const BIGNUM *a); 
void bn_set_static_words(BIGNUM *a, BN_ULONG *words, int size);



/*

 * Copy data into the BIGNUM. The caller must check that dmax is sufficient to

 * hold the data

 * Copy words into the BIGNUM |a|, reallocating space as necessary.

 * The negative flag of |a| is not modified.

 * Returns 1 on success and 0 on failure.

 */

void bn_set_data(BIGNUM *a, const void *data, size_t size);

/*

 * |num_words| is int because bn_expand2 takes an int. This is an internal

 * function so we simply trust callers not to pass negative values.

 */

int bn_set_words(BIGNUM *a, BN_ULONG *words, int num_words);



size_t bn_sizeof_BIGNUM(void);

include/openssl/bn.h

+1 −0
Original line number Diff line number Diff line
@@ -730,6 +730,7 @@ void ERR_load_BN_strings(void); 
# define BN_F_BN_NEW                                      113

# define BN_F_BN_RAND                                     114

# define BN_F_BN_RAND_RANGE                               122

# define BN_F_BN_SET_WORDS                                144

# define BN_F_BN_USUB                                     115



/* Reason codes. */