Skip to content

Commit

Permalink
pflog: use nd_ types in struct pfloghdr.
Browse files Browse the repository at this point in the history
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 the-tcpdump-group#1054.  (The SNMP issues are fixed by changes for the-tcpdump-group#1012.)

(cherry picked from commit a0b7859)
  • Loading branch information
guyharris authored and fxlb committed Oct 13, 2023
1 parent 33dd94c commit 5f60f0f
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 29 deletions.
40 changes: 20 additions & 20 deletions pflog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
19 changes: 10 additions & 9 deletions print-pflog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(": ");
}
Expand All @@ -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);
Expand All @@ -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;
Expand Down

0 comments on commit 5f60f0f

Please sign in to comment.