Hi,
this commit seems wrong to me.
The function verify_callback() in the file s_cb.c
contains this code:
switch (err) {
/* ... */
case X509_V_ERR_NO_EXPLICIT_POLICY:
policies_print(bio_err, ctx);
break;
}
if (err == X509_V_OK && ok == 2)
policies_print(bio_err, ctx);
BIO_printf(bio_err, "verify return:%d\n", ok);
So if the "case" or the "if" should ever trigger, that's a flat-out
use after free, unless i'm missing something.
I'm not sure the "case" or "if" *can* ever trigger, but if they
cannot, than these two calls should be removed instead, along with
the bio_err argument of policies_print().
When looking at a compiler warning, do not just blindly silence
the warning, but investigate the root cause instead.
Yours,
Ingo
--
Izproti problemu, pirms risini problemu.
-- Stanislavs Aloizs Borbals (1907-2000)
Rob Pierce wrote on Thu, Aug 16, 2018 at 06:28:35AM -0400:
> On Thu, Aug 16, 2018 at 06:14:06PM +0800, Nan Xiao wrote:
>> Hi tech@,
>>
>> The `free_out' variable seems redundant, so this patch removes it:
>>
>> Index: apps.c
>> ===================================================================
>> RCS file: /cvs/src/usr.bin/openssl/apps.c,v
>> retrieving revision 1.47
>> diff -u -p -r1.47 apps.c
>> --- apps.c 7 Feb 2018 08:57:25 -0000 1.47
>> +++ apps.c 16 Aug 2018 09:18:43 -0000
>> @@ -2050,11 +2050,9 @@ policies_print(BIO *out, X509_STORE_CTX
>> {
>> X509_POLICY_TREE *tree;
>> int explicit_policy;
>> - int free_out = 0;
>>
>> if (out == NULL) {
>> out = BIO_new_fp(stderr, BIO_NOCLOSE);
>> - free_out = 1;
>> }
>> tree = X509_STORE_CTX_get0_policy_tree(ctx);
>> explicit_policy = X509_STORE_CTX_get_explicit_policy(ctx);
> Committed. Thanks!
> Rob
