[openssl-dev] [openssl.org #4185] Bug in EVP_MD_CTX_copy_ex's malloc failure handling

[Richard Levitte via RT]

Considering we just had a substantial change in digest.c et al, inspiration is
the way to go. I figured that these two lines after the first memcpy() would be
good enough, as those are the variables that get populated afterward:

out->md_data = NULL; out->pctx = NULL;
Cheers,
Richard
Vid Thu, 17 Dec 2015 kl. 19.58.49, skrev davidben at google.com:
> On Thu, Dec 17, 2015 at 2:43 PM Kurt Roeckx via RT <rt at openssl.org>
> wrote:
>
> > On Wed, Dec 16, 2015 at 11:34:56PM +0000, David Benjamin via RT
> > wrote:
> > > EVP_MD_CTX_copy_ex is implemented with memcpy, followed by manually
> > fixing
> > > up |out->pctx| and |out->md_data|.
> > >
> > >
> >
https://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=crypto/evp/digest.c;h=5da0e01039a6da039942db9f1bf8b70753f509f2;hb=HEAD#l292
> > >
> > > If allocating |out->md_data| fails, then both |out->pctx| and |in-
> > > >pctx|
> > > may point to the same EVP_PKEY_CTX and freeing |out| will cause
> > > problems.
> > >
> > > We fixed this by not using memcpy.
> > >
> >
https://boringssl.googlesource.com/boringssl/+/306ece31bcaaed49e0240a2e5555f8901ebb2d45%5E%21/crypto/digest/digest.c
> >
> > This patch won't apply as is since we have more fields (engine,
> > flags).
> >
> > We also don't have pctx_ops, but have update instead, but already
> > seem to copy that anyway.
> >
>
> Right, we've diverged enough at this point that patches not applying
> as-is
> is the norm. :-) That was meant as a reference, but someone will need
> to do
> the equivalent change in OpenSSL if you all like the approach.
>
> David


--
Richard Levitte
levitte at openssl.org