On Fri, Jan 07, 2022 at 04:32:01PM -0800, Zhihong Yu wrote: > In contrib/pgcrypto/pgcrypto.c : > > err = px_combo_init(c, (uint8 *) VARDATA_ANY(key), klen, NULL, 0); > > Note: NULL is passed as iv. > > When combo_init() is called, > > if (ivlen > ivs) > memcpy(ivbuf, iv, ivs); > else > memcpy(ivbuf, iv, ivlen); > > It seems we need to consider the case of null being passed as iv for > memcpy() because of this: > > /usr/include/string.h:44:28: note: nonnull attribute specified here
I agree it's time to fix cases like this, given https://postgr.es/m/flat/20200904023648.gb3426...@rfd.leadboat.com. However, it should be one patch fixing all (or at least many) of them. > --- a/contrib/pgcrypto/px.c > +++ b/contrib/pgcrypto/px.c > @@ -198,10 +198,13 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned > klen, > if (ivs > 0) > { > ivbuf = palloc0(ivs); > - if (ivlen > ivs) > - memcpy(ivbuf, iv, ivs); > - else > - memcpy(ivbuf, iv, ivlen); > + if (iv != NULL) > + { > + if (ivlen > ivs) > + memcpy(ivbuf, iv, ivs); > + else > + memcpy(ivbuf, iv, ivlen); > + } > } If someone were to pass NULL iv with nonzero ivlen, that will silently malfunction. I'd avoid that risk by writing this way: --- a/contrib/pgcrypto/px.c +++ b/contrib/pgcrypto/px.c @@ -202,3 +202,3 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen, memcpy(ivbuf, iv, ivs); - else + else if (ivlen > 0) memcpy(ivbuf, iv, ivlen); That also gives the compiler an additional optimization strategy.
