From: Bernd Edlinger Date: Mon, 19 Mar 2018 13:20:53 +0000 (+0100) Subject: Fix bio callback backward compatibility X-Git-Tag: OpenSSL_1_1_1-pre3~30 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=c911e5da3c8e09a9531149c95aa92c65ecdf4b99;p=oweals%2Fopenssl.git 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: Rich Salz (Merged from https://github.com/openssl/openssl/pull/5516) --- diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c index 557d609cd1..95eef7d4bf 100644 --- a/crypto/bio/bio_lib.c +++ b/crypto/bio/bio_lib.c @@ -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 (*processed > INT_MAX) - return -1; - inret = *processed; - } + 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; } diff --git a/doc/man3/BIO_set_callback.pod b/doc/man3/BIO_set_callback.pod index 71a041ebf5..0a9b6edb65 100644 --- a/doc/man3/BIO_set_callback.pod +++ b/doc/man3/BIO_set_callback.pod @@ -114,7 +114,7 @@ is called before the free operation. =item B - 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 - 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 == B is special, because B is not the +argument of type B itself. In this case B is a pointer to +the actual call parameter, see B. + =back =head1 EXAMPLE diff --git a/test/bio_callback_test.c b/test/bio_callback_test.c new file mode 100644 index 0000000000..f1712b3a59 --- /dev/null +++ b/test/bio_callback_test.c @@ -0,0 +1,117 @@ +/* + * 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 +#include +#include + +#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; +} diff --git a/test/build.info b/test/build.info index 9fcaa7d8c8..a4e6e78b60 100644 --- a/test/build.info +++ b/test/build.info @@ -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 diff --git a/test/recipes/04-test_bio_callback.t b/test/recipes/04-test_bio_callback.t new file mode 100644 index 0000000000..1422cb6e21 --- /dev/null +++ b/test/recipes/04-test_bio_callback.t @@ -0,0 +1,12 @@ +#! /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");