Commit b3c31a65 authored by Bernd Edlinger's avatar Bernd Edlinger Committed by Richard Levitte
Browse files

Fix the error handling in CRYPTO_dup_ex_data. 


Fix a strict aliasing issue in ui_dup_method_data.
Add test coverage for CRYPTO_dup_ex_data, use OPENSSL_assert.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2988)
parent e41e5d1e

crypto/ex_data.c

+14 −6
Original line number Diff line number Diff line
@@ -124,7 +124,7 @@ static int dummy_dup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from, 
                     void *from_d, int idx,

                     long argl, void *argp)

{

    return 0;

    return 1;

}



int CRYPTO_free_ex_index(int class_index, int idx)
@@ -254,10 +254,11 @@ int CRYPTO_dup_ex_data(int class_index, CRYPTO_EX_DATA *to, 
                       const CRYPTO_EX_DATA *from)

{

    int mx, j, i;

    char *ptr;

    void *ptr;

    EX_CALLBACK *stack[10];

    EX_CALLBACK **storage = NULL;

    EX_CALLBACKS *ip;

    int toret = 0;



    if (from->sk == NULL)

        /* Nothing to copy over */
@@ -280,21 +281,28 @@ int CRYPTO_dup_ex_data(int class_index, CRYPTO_EX_DATA *to, 
    }

    CRYPTO_THREAD_unlock(ex_data_lock);



    if (mx > 0 && storage == NULL) {

    if (mx == 0)

        return 1;

    if (storage == NULL) {

        CRYPTOerr(CRYPTO_F_CRYPTO_DUP_EX_DATA, ERR_R_MALLOC_FAILURE);

        return 0;

    }

    if (!CRYPTO_set_ex_data(to, mx - 1, NULL))

        goto err;



    for (i = 0; i < mx; i++) {

        ptr = CRYPTO_get_ex_data(from, i);

        if (storage[i] && storage[i]->dup_func)

            storage[i]->dup_func(to, from, &ptr, i,

                                 storage[i]->argl, storage[i]->argp);

            if (!storage[i]->dup_func(to, from, &ptr, i,

                                      storage[i]->argl, storage[i]->argp))

                goto err;

        CRYPTO_set_ex_data(to, i, ptr);

    }

    toret = 1;

 err:

    if (storage != stack)

        OPENSSL_free(storage);

    return 1;

    return toret;

}

doc/man3/CRYPTO_get_ex_new_index.pod

+7 −4
Original line number Diff line number Diff line
@@ -130,12 +130,15 @@ the same callback handles different types of exdata. 
dup_func() is called when a structure is being copied.  This is only done

for B<SSL> and B<SSL_SESSION> objects.  The B<to> and B<from> parameters

are pointers to the destination and source B<CRYPTO_EX_DATA> structures,

respectively.  The B<srcp> parameter is a pointer to the source exdata.

When the dup_func() returns, the value in B<srcp> is copied to the

destination ex_data.  If the pointer contained in B<srcp> is not modified

respectively.  The B<from_d> parameter needs to be cast to a B<void **pptr>

as the API has currently the wrong signature; that will be changed in a

future version.  The B<*pptr> is a pointer to the source exdata.

When the dup_func() returns, the value in B<*pptr> is copied to the

destination ex_data.  If the pointer contained in B<*pptr> is not modified

by the dup_func(), then both B<to> and B<from> will point to the same data.

The B<idx>, B<argl> and B<argp> parameters are as described for the other

two callbacks.

two callbacks.  If the dup_func() returns B<0> the whole CRYPTO_dup_ex_data()

will fail.



=head1 RETURN VALUES

include/openssl/crypto.h

+1 −1
Original line number Diff line number Diff line
@@ -175,7 +175,7 @@ typedef void CRYPTO_EX_new (void *parent, void *ptr, CRYPTO_EX_DATA *ad, 
typedef void CRYPTO_EX_free (void *parent, void *ptr, CRYPTO_EX_DATA *ad,

                             int idx, long argl, void *argp);

typedef int CRYPTO_EX_dup (CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,

                           void *srcp, int idx, long argl, void *argp);

                           void *from_d, int idx, long argl, void *argp);

__owur int CRYPTO_get_ex_new_index(int class_index, long argl, void *argp,

                            CRYPTO_EX_new *new_func, CRYPTO_EX_dup *dup_func,

                            CRYPTO_EX_free *free_func);

test/exdatatest.c

+34 −16
Original line number Diff line number Diff line
@@ -10,7 +10,6 @@ 
#include <stdio.h>

#include <string.h>

#include <stdlib.h>

#include <assert.h>

#include <openssl/crypto.h>



static long saved_argl;
@@ -20,26 +19,28 @@ static int saved_idx; 
static void exnew(void *parent, void *ptr, CRYPTO_EX_DATA *ad,

          int idx, long argl, void *argp)

{

    assert(idx == saved_idx);

    assert(argl == saved_argl);

    assert(argp == saved_argp);

    OPENSSL_assert(idx == saved_idx);

    OPENSSL_assert(argl == saved_argl);

    OPENSSL_assert(argp == saved_argp);

    OPENSSL_assert(ptr == NULL);

}



static int exdup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,

          void *from_d, int idx, long argl, void *argp)

{

    assert(idx == saved_idx);

    assert(argl == saved_argl);

    assert(argp == saved_argp);

    return 0;

    OPENSSL_assert(idx == saved_idx);

    OPENSSL_assert(argl == saved_argl);

    OPENSSL_assert(argp == saved_argp);

    OPENSSL_assert(from_d != NULL);

    return 1;

}



static void exfree(void *parent, void *ptr, CRYPTO_EX_DATA *ad,

            int idx, long argl, void *argp)

{

    assert(idx == saved_idx);

    assert(argl == saved_argl);

    assert(argp == saved_argp);

    OPENSSL_assert(idx == saved_idx);

    OPENSSL_assert(argl == saved_argl);

    OPENSSL_assert(argp == saved_argp);

}



typedef struct myobj_st {
@@ -55,14 +56,14 @@ static MYOBJ *MYOBJ_new() 


    obj->id = ++count;

    obj->st = CRYPTO_new_ex_data(CRYPTO_EX_INDEX_APP, obj, &obj->ex_data);

    assert(obj->st != 0);

    OPENSSL_assert(obj->st != 0);

    return obj;

}



static void MYOBJ_sethello(MYOBJ *obj, char *cp)

{

    obj->st = CRYPTO_set_ex_data(&obj->ex_data, saved_idx, cp);

    assert(obj->st != 0);

    OPENSSL_assert(obj->st != 0);

}



static char *MYOBJ_gethello(MYOBJ *obj)
@@ -76,9 +77,19 @@ static void MYOBJ_free(MYOBJ *obj) 
    OPENSSL_free(obj);

}



static MYOBJ *MYOBJ_dup(MYOBJ *in)

{

    MYOBJ *obj = MYOBJ_new();



    obj->st = CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_APP, &obj->ex_data,

                                 &in->ex_data);

    OPENSSL_assert(obj->st != 0);

    return obj;

}



int main()

{

    MYOBJ *t1, *t2;

    MYOBJ *t1, *t2, *t3;

    const char *cp;

    char *p;
@@ -92,15 +103,22 @@ int main() 
    t2 = MYOBJ_new();

    MYOBJ_sethello(t1, p);

    cp = MYOBJ_gethello(t1);

    assert(cp == p);

    OPENSSL_assert(cp == p);

    if (cp != p)

        return 1;

    cp = MYOBJ_gethello(t2);

    assert(cp == NULL);

    OPENSSL_assert(cp == NULL);

    if (cp != NULL)

        return 1;

    t3 = MYOBJ_dup(t1);

    cp = MYOBJ_gethello(t3);

    OPENSSL_assert(cp == p);

    if (cp != p)

        return 1;

    cp = MYOBJ_gethello(t2);

    MYOBJ_free(t1);

    MYOBJ_free(t2);

    MYOBJ_free(t3);

    free(saved_argp);

    free(p);

    return 0;