On Mon, Jul 19, 2021 at 4:41 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jul 19, 2021 at 9:19 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > On Mon, Jul 19, 2021 at 12:43 PM Amit Kapila <amit.kapil...@gmail.com> > > wrote: > > > > > > On Mon, Jul 19, 2021 at 1:55 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > > > > > Amit Kapila <amit.kapil...@gmail.com> writes: > > > > > Pushed. > > > > > > > > I think you'd be way better off making the gid fields be "char *" > > > > and pstrdup'ing the result of pq_getmsgstring. Another possibility > > > > perhaps is to use strlcpy, but I'd only go that way if it's important > > > > to constrain the received strings to 200 bytes. > > > > > > > > > > I think it is important to constrain length to 200 bytes for this case > > > as here we receive a prepared transaction identifier which according > > > to docs [1] has a max length of 200 bytes. Also, in > > > ParseCommitRecord() and ParseAbortRecord(), we are using strlcpy with > > > 200 as max length to copy prepare transaction identifier. So, I think > > > it is better to use strlcpy here unless you or Peter feels otherwise. > > > > > > > OK. I have implemented this reported [1] potential buffer overrun > > using the constraining strlcpy, because the GID limitation of 200 > > bytes is already mentioned in the documentation [2]. > > > > This will work but I think it is better to use sizeof gid buffer as we > are using in ParseCommitRecord() and ParseAbortRecord(). Tomorrow, if > due to some unforeseen reason if we change the size of gid buffer to > be different than the GIDSIZE then it will work seamlessly. >
Modified as requested. PSA patch v2. ------ Kind Regards, Peter Smith. Fujitsu Australia
v2-0001-Fix-potential-buffer-overruns.patch
Description: Binary data
