Commit c911e5da authored by Bernd Edlinger's avatar Bernd Edlinger
Browse files

Fix bio callback backward compatibility 



Don't pass a pointer to uninitialized processed value
for BIO_CB_READ and BIO_CB_WRITE

Check the correct cmd code in BIO_callback_ctrl

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5516)
parent d4ef4fbf

crypto/bio/bio_lib.c

+11 −11
Original line number Diff line number Diff line
@@ -34,9 +34,8 @@ static long bio_call_callback(BIO *b, int oper, const char *argp, size_t len, 
    long ret;

    int bareoper;



    if (b->callback_ex != NULL) {

    if (b->callback_ex != NULL)

        return b->callback_ex(b, oper, argp, len, argi, argl, inret, processed);

    }



    /* Strip off any BIO_CB_RETURN flag */

    bareoper = oper & ~BIO_CB_RETURN;
@@ -51,17 +50,17 @@ static long bio_call_callback(BIO *b, int oper, const char *argp, size_t len, 
            return -1;



        argi = (int)len;

    }



        if (inret && (oper & BIO_CB_RETURN)) {

    if (inret && (oper & BIO_CB_RETURN) && bareoper != BIO_CB_CTRL) {

        if (*processed > INT_MAX)

            return -1;

        inret = *processed;

    }

    }



    ret = b->callback(b, oper, argp, argi, argl, inret);



    if (ret >= 0 && (HAS_LEN_OPER(bareoper) || bareoper == BIO_CB_PUTS)) {

    if (ret >= 0 && (oper & BIO_CB_RETURN) && bareoper != BIO_CB_CTRL) {

        *processed = (size_t)ret;

        ret = 1;

    }
@@ -260,7 +259,7 @@ static int bio_read_intern(BIO *b, void *data, size_t dlen, size_t *readbytes) 


    if ((b->callback != NULL || b->callback_ex != NULL) &&

        ((ret = (int)bio_call_callback(b, BIO_CB_READ, data, dlen, 0, 0L, 1L,

                                       readbytes)) <= 0))

                                       NULL)) <= 0))

        return ret;



    if (!b->init) {
@@ -333,7 +332,7 @@ static int bio_write_intern(BIO *b, const void *data, size_t dlen, 


    if ((b->callback != NULL || b->callback_ex != NULL) &&

        ((ret = (int)bio_call_callback(b, BIO_CB_WRITE, data, dlen, 0, 0L, 1L,

                                       written)) <= 0))

                                       NULL)) <= 0))

        return ret;



    if (!b->init) {
@@ -542,7 +541,8 @@ long BIO_callback_ctrl(BIO *b, int cmd, BIO_info_cb *fp) 
    if (b == NULL)

        return 0;



    if ((b->method == NULL) || (b->method->callback_ctrl == NULL)) {

    if ((b->method == NULL) || (b->method->callback_ctrl == NULL)

            || (cmd != BIO_CTRL_SET_CALLBACK)) {

        BIOerr(BIO_F_BIO_CALLBACK_CTRL, BIO_R_UNSUPPORTED_METHOD);

        return -2;

    }

doc/man3/BIO_set_callback.pod

+11 −7
Original line number Diff line number Diff line
@@ -114,7 +114,7 @@ is called before the free operation. 


=item B<BIO_read_ex(b, data, dlen, readbytes)>



 callback_ex(b, BIO_CB_READ, data, dlen, 0, 0L, 1L, readbytes)

 callback_ex(b, BIO_CB_READ, data, dlen, 0, 0L, 1L, NULL)



or
@@ -123,7 +123,7 @@ or 
is called before the read and



 callback_ex(b, BIO_CB_READ | BIO_CB_RETURN, data, dlen, 0, 0L, retvalue,

             readbytes)

             &readbytes)



or
@@ -133,7 +133,7 @@ after. 


=item B<BIO_write(b, data, dlen, written)>



 callback_ex(b, BIO_CB_WRITE, data, dlen, 0, 0L, 1L, written)

 callback_ex(b, BIO_CB_WRITE, data, dlen, 0, 0L, 1L, NULL)



or
@@ -142,7 +142,7 @@ or 
is called before the write and



 callback_ex(b, BIO_CB_WRITE | BIO_CB_RETURN, data, dlen, 0, 0L, retvalue,

             written)

             &written)



or
@@ -161,7 +161,7 @@ or 
is called before the operation and



 callback_ex(b, BIO_CB_GETS | BIO_CB_RETURN, buf, size, 0, 0L, retvalue,

             readbytes)

             &readbytes)



or
@@ -179,11 +179,11 @@ or 


is called before the operation and



 callback_ex(b, BIO_CB_PUTS | BIO_CB_RETURN, buf, 0, 0, 0L, retvalue, written)

 callback_ex(b, BIO_CB_PUTS | BIO_CB_RETURN, buf, 0, 0, 0L, retvalue, &written)



or



 callback(b, BIO_CB_WRITE|BIO_CB_RETURN, buf, 0, 0L, retvalue)

 callback(b, BIO_CB_PUTS|BIO_CB_RETURN, buf, 0, 0L, retvalue)



after.
@@ -205,6 +205,10 @@ or 


after.



Note: B<cmd> == B<BIO_CTRL_SET_CALLBACK> is special, because B<parg> is not the

argument of type B<BIO_info_cb> itself.  In this case B<parg> is a pointer to

the actual call parameter, see B<BIO_callback_ctrl>.



=back



=head1 EXAMPLE

test/bio_callback_test.c

0 → 100644
+117 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.

 *

 * Licensed under the OpenSSL license (the "License").  You may not use

 * this file except in compliance with the License.  You can obtain a copy

 * in the file LICENSE in the source distribution or at

 * https://www.openssl.org/source/license.html

 */

#include <stdio.h>

#include <string.h>

#include <openssl/bio.h>



#include "testutil.h"



#define MAXCOUNT 5

static int         my_param_count;

static BIO        *my_param_b[MAXCOUNT];

static int         my_param_oper[MAXCOUNT];

static const char *my_param_argp[MAXCOUNT];

static int         my_param_argi[MAXCOUNT];

static long        my_param_argl[MAXCOUNT];

static long        my_param_ret[MAXCOUNT];



static long my_bio_callback(BIO *b, int oper, const char *argp, int argi,

                            long argl, long ret)

{

    if (my_param_count >= MAXCOUNT)

        return -1;

    my_param_b[my_param_count]    = b;

    my_param_oper[my_param_count] = oper;

    my_param_argp[my_param_count] = argp;

    my_param_argi[my_param_count] = argi;

    my_param_argl[my_param_count] = argl;

    my_param_ret[my_param_count]  = ret;

    my_param_count++;

    return ret;

}



static int test_bio_callback(void)

{

    int ok = 0;

    BIO *bio;

    int i;

    char *test1 = "test";

    char *test2 = "hello";



    my_param_count = 0;



    bio = BIO_new(BIO_s_mem());

    if (bio == NULL)

        goto err;



    BIO_set_callback(bio, my_bio_callback);

    i = BIO_write(bio, test1, 4);

    if (!TEST_int_eq(i, 4)

            || !TEST_int_eq(my_param_count, 2)

            || !TEST_ptr_eq(my_param_b[0], bio)

            || !TEST_int_eq(my_param_oper[0], BIO_CB_WRITE)

            || !TEST_ptr_eq(my_param_argp[0], test1)

            || !TEST_int_eq(my_param_argi[0], 4)

            || !TEST_long_eq(my_param_argl[0], 0L)

            || !TEST_long_eq(my_param_ret[0], 1L)

            || !TEST_ptr_eq(my_param_b[1], bio)

            || !TEST_int_eq(my_param_oper[1], BIO_CB_WRITE | BIO_CB_RETURN)

            || !TEST_ptr_eq(my_param_argp[1], test1)

            || !TEST_int_eq(my_param_argi[1], 4)

            || !TEST_long_eq(my_param_argl[1], 0L)

            || !TEST_long_eq(my_param_ret[1], 4L))

        goto err;



    i = BIO_puts(bio, test2);

    if (!TEST_int_eq(i, 5)

            || !TEST_int_eq(my_param_count, 4)

            || !TEST_ptr_eq(my_param_b[2], bio)

            || !TEST_int_eq(my_param_oper[2], BIO_CB_PUTS)

            || !TEST_ptr_eq(my_param_argp[2], test2)

            || !TEST_int_eq(my_param_argi[2], 0)

            || !TEST_long_eq(my_param_argl[2], 0L)

            || !TEST_long_eq(my_param_ret[2], 1L)

            || !TEST_ptr_eq(my_param_b[3], bio)

            || !TEST_int_eq(my_param_oper[3], BIO_CB_PUTS | BIO_CB_RETURN)

            || !TEST_ptr_eq(my_param_argp[3], test2)

            || !TEST_int_eq(my_param_argi[3], 0)

            || !TEST_long_eq(my_param_argl[3], 0L)

            || !TEST_long_eq(my_param_ret[3], 5L))

        goto err;



    i = BIO_free(bio);



    if (!TEST_int_eq(i, 1)

            || !TEST_int_eq(my_param_count, 5)

            || !TEST_ptr_eq(my_param_b[4], bio)

            || !TEST_int_eq(my_param_oper[4], BIO_CB_FREE)

            || !TEST_ptr_eq(my_param_argp[4], NULL)

            || !TEST_int_eq(my_param_argi[4], 0)

            || !TEST_long_eq(my_param_argl[4], 0L)

            || !TEST_long_eq(my_param_ret[4], 1L))

        goto finish;



    ok = 1;

    goto finish;



err:

    BIO_free(bio);



finish:

    /* This helps finding memory leaks with ASAN */

    memset(my_param_b, 0, sizeof(my_param_b));

    memset(my_param_argp, 0, sizeof(my_param_argp));

    return ok;

}



int setup_tests(void)

{

    ADD_TEST(test_bio_callback);

    return 1;

}

test/build.info

+5 −0
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=MAIN 
          packettest asynctest secmemtest srptest memleaktest stack_test \

          dtlsv1listentest ct_test threadstest afalgtest d2i_test \

          ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \

          bio_callback_test \

          bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \

          pkey_meth_test pkey_meth_kdf_test uitest cipherbytes_test \

          asn1_encode_test asn1_string_table_test \
@@ -280,6 +281,10 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=MAIN 
  INCLUDE[asynciotest]=../include

  DEPEND[asynciotest]=../libcrypto ../libssl libtestutil.a



  SOURCE[bio_callback_test]=bio_callback_test.c

  INCLUDE[bio_callback_test]=../include

  DEPEND[bio_callback_test]=../libcrypto libtestutil.a



  SOURCE[bioprinttest]=bioprinttest.c

  INCLUDE[bioprinttest]=../include

  DEPEND[bioprinttest]=../libcrypto libtestutil.a

test/recipes/04-test_bio_callback.t

0 → 100644
+12 −0
Original line number Diff line number Diff line 
#! /usr/bin/env perl

# Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.

#

# Licensed under the OpenSSL license (the "License").  You may not use

# this file except in compliance with the License.  You can obtain a copy

# in the file LICENSE in the source distribution or at

# https://www.openssl.org/source/license.html





use OpenSSL::Test::Simple;



simple_test("test_bio_callback", "bio_callback_test");