On Sat, Jan 08, 2022 at 06:52:14PM -0800, Zhihong Yu wrote: > On Sat, Jan 8, 2022 at 5:52 PM Noah Misch <n...@leadboat.com> wrote: > > On Fri, Jan 07, 2022 at 04:32:01PM -0800, Zhihong Yu wrote:
> > 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 > > > Hi, > If iv is NULL, none of the memcpy() would be called (based on my patch). > Can you elaborate your suggestion in more detail ? On further thought, I would write it 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 helps in two ways. First, if someone passes iv==NULL and ivlen!=0, my version will tend to crash, but yours will treat that like ivlen==0. Since this would be a programming error, crashing is better. Second, a compiler can opt to omit the "ivlen != 0" test from the generated assembly, because the compiler can know that memcpy(any_value_a, any_value_b, 0) is a no-op. > Since the referenced email was old, line numbers have changed. > It would be nice if an up-to-date list is provided in case more places > should be changed. To check whether you've gotten them all, configure with CC='gcc -fsanitize=undefined -fsanitize-undefined-trap-on-error' and run check-world.
