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 #1054.  (The SNMP issues are fixed by changes for #1012.)
  • Loading branch information
guyharris committed Aug 21, 2023
1 parent a028658 commit a0b7859
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 a0b7859

Please sign in to comment.