From 5f60f0fcabd8e85eda2446b75854f09812afc12b Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Mon, 21 Aug 2023 13:37:44 -0700 Subject: [PATCH] pflog: use nd_ types in struct pfloghdr. This 1) makes sure that GET_ macros are used to extract data from the structure (which they already were) and 2) avoids undefined behavior if the structure isn't aligned on the appropriate memory boundary. Fixes #1054. (The SNMP issues are fixed by changes for #1012.) (cherry picked from commit a0b785957c77e014b566949bf00ff746e2071813) --- pflog.h | 40 ++++++++++++++++++++-------------------- print-pflog.c | 19 ++++++++++--------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/pflog.h b/pflog.h index a629ebabf..6680b4246 100644 --- a/pflog.h +++ b/pflog.h @@ -115,35 +115,35 @@ struct pf_addr { }; struct pfloghdr { - uint8_t length; - uint8_t af; - uint8_t action; - uint8_t reason; + nd_uint8_t length; + nd_uint8_t af; + nd_uint8_t action; + nd_uint8_t reason; char ifname[PFLOG_IFNAMSIZ]; char ruleset[PFLOG_RULESET_NAME_SIZE]; - uint32_t rulenr; - uint32_t subrulenr; - uint32_t uid; - int32_t pid; - uint32_t rule_uid; - int32_t rule_pid; - uint8_t dir; + nd_uint32_t rulenr; + nd_uint32_t subrulenr; + nd_uint32_t uid; + nd_int32_t pid; + nd_uint32_t rule_uid; + nd_int32_t rule_pid; + nd_uint8_t dir; #if defined(__OpenBSD__) - uint8_t rewritten; - uint8_t naf; - uint8_t pad[1]; + nd_uint8_t rewritten; + nd_uint8_t naf; + nd_uint8_t pad[1]; #else - uint8_t pad[3]; + nd_uint8_t pad[3]; #endif #if defined(__FreeBSD__) - uint32_t ridentifier; - uint8_t reserve; - uint8_t pad2[3]; + nd_uint32_t ridentifier; + nd_uint8_t reserve; + nd_uint8_t pad2[3]; #elif defined(__OpenBSD__) struct pf_addr saddr; struct pf_addr daddr; - uint16_t sport; - uint16_t dport; + nd_uint16_t sport; + nd_uint16_t dport; #endif }; diff --git a/print-pflog.c b/print-pflog.c index 1cffbf6bf..9424c9e07 100644 --- a/print-pflog.c +++ b/print-pflog.c @@ -106,8 +106,8 @@ pflog_print(netdissect_options *ndo, const struct pfloghdr *hdr) uint32_t rulenr, subrulenr; ndo->ndo_protocol = "pflog"; - rulenr = GET_BE_U_4(&hdr->rulenr); - subrulenr = GET_BE_U_4(&hdr->subrulenr); + rulenr = GET_BE_U_4(hdr->rulenr); + subrulenr = GET_BE_U_4(hdr->subrulenr); if (subrulenr == (uint32_t)-1) ND_PRINT("rule %u/", rulenr); else { @@ -117,9 +117,9 @@ pflog_print(netdissect_options *ndo, const struct pfloghdr *hdr) } ND_PRINT("%s: %s %s on ", - tok2str(pf_reasons, "unkn(%u)", GET_U_1(&hdr->reason)), - tok2str(pf_actions, "unkn(%u)", GET_U_1(&hdr->action)), - tok2str(pf_directions, "unkn(%u)", GET_U_1(&hdr->dir))); + tok2str(pf_reasons, "unkn(%u)", GET_U_1(hdr->reason)), + tok2str(pf_actions, "unkn(%u)", GET_U_1(hdr->action)), + tok2str(pf_directions, "unkn(%u)", GET_U_1(hdr->dir))); nd_printjnp(ndo, (const u_char*)hdr->ifname, PFLOG_IFNAMSIZ); ND_PRINT(": "); } @@ -144,12 +144,13 @@ pflog_if_print(netdissect_options *ndo, const struct pcap_pkthdr *h, #define MIN_PFLOG_HDRLEN 45 hdr = (const struct pfloghdr *)p; - if (GET_U_1(&hdr->length) < MIN_PFLOG_HDRLEN) { + hdrlen = GET_U_1(hdr->length); + if (hdrlen < MIN_PFLOG_HDRLEN) { ND_PRINT("[pflog: invalid header length!]"); - ndo->ndo_ll_hdr_len += GET_U_1(&hdr->length); /* XXX: not really */ + ndo->ndo_ll_hdr_len += hdrlen; /* XXX: not really */ return; } - hdrlen = roundup2(hdr->length, 4); + hdrlen = roundup2(hdrlen, 4); if (caplen < hdrlen) { nd_print_trunc(ndo); @@ -163,7 +164,7 @@ pflog_if_print(netdissect_options *ndo, const struct pcap_pkthdr *h, pflog_print(ndo, hdr); /* skip to the real packet */ - af = GET_U_1(&hdr->af); + af = GET_U_1(hdr->af); length -= hdrlen; caplen -= hdrlen; p += hdrlen;