From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Kapila <amit.kapila16@gmail.com>
cc: Peter Smith <smithpb2250@gmail.com>, vignesh C <vignesh21@gmail.com>,
        "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>,
        Ajin Cherian <itsajin@gmail.com>,
 Greg Nancarrow <gregn4422@gmail.com>,
        PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] logical decoding of two-phase transactions
In-reply-to: 
 <CAA4eK1Lt2SfanC-mTq-oM1-sZon1zF43Vc5YcXccpa1aNm0V3g@mail.gmail.com>
References: 
 <CAA4eK1Jz64rwLyB6H7Z_SmEDouJ41KN42=VkVFp6JTpafJFG8Q@mail.gmail.com>
 <CAHut+PucbsUzfiaFzi=TVLs0R5zf6+9x-YtJZgLtB1zxZtqHQg@mail.gmail.com>
 <OS0PR01MB611332752AFF6ADF580AB849FB259@OS0PR01MB6113.jpnprd01.prod.outlook.com>
 <CAFPTHDYx3_jrpFE25NuBONaoybiZw4VsSYMm_3CCY_1zhrsfAQ@mail.gmail.com>
 <OS0PR01MB6113533E9A6EAE8EC7B76BA5FB239@OS0PR01MB6113.jpnprd01.prod.outlook.com>
 <CAFPTHDYF=OY5QBVu2O01vPRRrVsxffi0qgKUxEeHtJ6cecMf7Q@mail.gmail.com>
 <CAA4eK1K7qhqigORdEgqFTOPfj4r2+jV-uLc4-RCtgyDZwvbF8w@mail.gmail.com>
 <CAA4eK1Jd9sqWtt5kEJZL1ehJB2y_DFnvDjY9vJ51k8Wq6XWVyw@mail.gmail.com>
 <CAHut+Pt7wnctZpfhaLyuPA0mXDAtuw7DsMUfw3TePJLxqTArjA@mail.gmail.com>
 <CAJcOf-cvn4EpSo4cD_9Awop72roKL1vnMtpURn1FnXv+gX5VPA@mail.gmail.com>
 <CAHut+Pvn_s7GvTD8AsPxxVTPSYn4sMZ89HG8LHBDuDA251ffMg@mail.gmail.com>
 <CAJcOf-fPcpe21RciPRn_56FwO6K_B+VcTZ2prAv4xvAk4cqYiQ@mail.gmail.com>
 <CAHut+PvOVkiVBf4P5chdVSoVs5=a=F_GtTSHHoXDb4LiOM_8Qw@mail.gmail.com>
 <CAHut+Pvdio4=OE6cz5pr8VcJNcAgt5uGBPdKf-tnGEMa1mANGg@mai!
 l.gmail.com>
 <CAJcOf-eDRLyqOCBwanisJ93fRpxr0_RsZgRPMBCqQjEdJV9zSg@mail.gmail.com>
 <CAHut+PuJKTNRjFre0VBufWMz9BEScC__nT+PUhbSaUNW2biPow@mail.gmail.com>
 <CAA4eK1JO3HsOurS988=Jarej=AK6ChE1tLuMNP=AZCt6--hVrw@mail.gmail.com>
 <CAHut+PuDGn4uTb9Z4-6SJA8zUpZAj6Q1MV1XET_=BUfXtYJtfQ@mail.gmail.com>
 <CAJcOf-cF4L2oFRSLN3VLhH-701rhQLc7KdTqOsh0hmxLy1_=UQ@mail.gmail.com>
 <CAFPTHDaLNcziUa-fbCzfuvKxUdDYALnkpU9yDuPuMJNgSqu_gg@mail.gmail.com>
 <CALDaNm0jNBLCmiO+XBGStC+q7hCCx=HYaqOAh8yHw6BZhuYzFQ@mail.gmail.com>
 <CAFPTHDb+Ph1rx7nQHJHTyFEYtNENPEo9s5ku760Uo3j9g3-rfQ@mail.gmail.com>
 <CAA4eK1L4jAceguqxonDb7_AOmaB6z0TCq233q+w=vEvFXn4cxw@mail.gmail.com>
 <CALDaNm1k0o-Q+WAKNWUk9pW_Sv7=cW06xA5txhg0rtpk6g-6aA@mail.gmail.com>
 <CAA4eK1LYz8oduZ4f7PueWFqSiSfDTTw1Ze+vogzNc2bH-PbfxQ@mail.gmail.com>
 <CAHut+PsydLNGzfOcSYLi1fbk+D5riA0ggzoKsqKXLvR2JWmDqg@mail.gmail.com>
 <CAFPTHDbQPR=-E+8jh7a_jbdTovYcR31Eoa_aAo6D4Wg6Lj_FhQ@mail.gmail.com>
 <OS0PR01MB6113239BE2404681AAB9E2F3FB1F9@OS0PR01MB6113.jpnprd01.prod.outlook.co!
 m> <CAHut+PsSZJsrJatRD9tfEGxcmRH4iek0osdW8LhpcFZF9_6ojg@mail.gmail.com>
 <CAA4eK1KQvXgW=eotJv_V_950fOk8NUDgmykGzg_WOqMzgEyp=Q@mail.gmail.com>
 <CALDaNm15ce8Qiz5WqaBMUyTMnQ=XCMz9NiAkh6LOo_+44uzi-A@mail.gmail.com>
 <CAHut+Psjio5gYMqSzphEPbEigie2iSMXjy4M2YoJ=6=N3UfZaQ@mail.gmail.com>
 <CAA4eK1LgSJ3XEFp37DgwUGJPYLeEkx_6=3ZmE0ZW0b0Gi
Comments: In-reply-to Amit Kapila <amit.kapila16@gmail.com>
	message dated "Wed, 14 Jul 2021 11:52:58 +0530"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <161028.1626639923.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 18 Jul 2021 16:25:23 -0400
Message-ID: <161029.1626639923@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/161029.1626639923%40sss.pgh.pa.us>
Precedence: bulk

Amit Kapila <amit.kapila16@gmail.com> writes:
> Pushed.

Coverity thinks this has security issues, and I agree.

/srv/coverity/git/pgsql-git/postgresql/src/backend/replication/logical/pro=
to.c: 144 in logicalrep_read_begin_prepare()
143     	/* read gid (copy it into a pre-allocated buffer) */
>>>     CID 1487517:  Security best practices violations  (STRING_OVERFLOW=
)
>>>     You might overrun the 200-character fixed-size string "begin_data-=
>gid" by copying the return value of "pq_getmsgstring" without checking th=
e length.
144     	strcpy(begin_data->gid, pq_getmsgstring(in));

200     	/* read gid (copy it into a pre-allocated buffer) */
>>>     CID 1487515:  Security best practices violations  (STRING_OVERFLOW=
)
>>>     You might overrun the 200-character fixed-size string "prepare_dat=
a->gid" by copying the return value of "pq_getmsgstring" without checking =
the length.
201     	strcpy(prepare_data->gid, pq_getmsgstring(in));

256     	/* read gid (copy it into a pre-allocated buffer) */
>>>     CID 1487516:  Security best practices violations  (STRING_OVERFLOW=
)
>>>     You might overrun the 200-character fixed-size string "prepare_dat=
a->gid" by copying the return value of "pq_getmsgstring" without checking =
the length.
257     	strcpy(prepare_data->gid, pq_getmsgstring(in));

316     	/* read gid (copy it into a pre-allocated buffer) */
>>>     CID 1487519:  Security best practices violations  (STRING_OVERFLOW=
)
>>>     You might overrun the 200-character fixed-size string "rollback_da=
ta->gid" by copying the return value of "pq_getmsgstring" without checking=
 the length.
317     	strcpy(rollback_data->gid, pq_getmsgstring(in));

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.

			regards, tom lane