On Mon, Nov 07, 2022 at 08:12:23PM +0100, Moritz Buhl wrote:
> Hi tech,
> Dear claudio,
>
> there could be an uninitialized stack memory access in pfkey_reply.
>
> It looks like this:
> struct sadb_msg hdr, *msg;
> ...
>
> do {
> rv = pfkey_read(sd, &hdr);
> if (rv == -1)
> return (-1);
> } while (rv);
>
> if (hdr.sadb_msg_errno != 0) {
>
> And in the errno cases of pfkey_read, it doesn't initialize hdr but
> then breaks out of the do-while loop.
>
> OK?
OK claudio@
> mbuhl
>
> Found by CodeChecker.
>
> Index: bgpd/pfkey.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/pfkey.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 pfkey.c
> --- bgpd/pfkey.c 17 Aug 2022 15:15:26 -0000 1.67
> +++ bgpd/pfkey.c 7 Nov 2022 18:58:37 -0000
> @@ -423,7 +423,7 @@ pfkey_read(int sd, struct sadb_msg *h)
>
> if (recv(sd, &hdr, sizeof(hdr), MSG_PEEK) != sizeof(hdr)) {
> if (errno == EAGAIN || errno == EINTR)
> - return (0);
> + return (1);
> log_warn("pfkey peek");
> return (-1);
> }
> @@ -439,7 +439,7 @@ pfkey_read(int sd, struct sadb_msg *h)
> /* not ours, discard */
> if (read(sd, &hdr, sizeof(hdr)) == -1) {
> if (errno == EAGAIN || errno == EINTR)
> - return (0);
> + return (1);
> log_warn("pfkey read");
> return (-1);
> }
> Index: ldpd/pfkey.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/pfkey.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 pfkey.c
> --- ldpd/pfkey.c 23 Jan 2019 02:02:04 -0000 1.12
> +++ ldpd/pfkey.c 7 Nov 2022 19:00:01 -0000
> @@ -253,7 +253,7 @@ pfkey_read(int sd, struct sadb_msg *h)
>
> if (recv(sd, &hdr, sizeof(hdr), MSG_PEEK) != sizeof(hdr)) {
> if (errno == EAGAIN || errno == EINTR)
> - return (0);
> + return (1);
> log_warn("pfkey peek");
> return (-1);
> }
> @@ -269,7 +269,7 @@ pfkey_read(int sd, struct sadb_msg *h)
> /* not ours, discard */
> if (read(sd, &hdr, sizeof(hdr)) == -1) {
> if (errno == EAGAIN || errno == EINTR)
> - return (0);
> + return (1);
> log_warn("pfkey read");
> return (-1);
> }
--
:wq Claudio
