Changeset View
Changeset View
Standalone View
Standalone View
lib/libpfctl/libpfctl.c
Show First 20 Lines • Show All 142 Lines • ▼ Show 20 Lines | |||||
static void | static void | ||||
pfctl_nv_add_addr_wrap(nvlist_t *nvparent, const char *name, | pfctl_nv_add_addr_wrap(nvlist_t *nvparent, const char *name, | ||||
const struct pf_addr_wrap *addr) | const struct pf_addr_wrap *addr) | ||||
{ | { | ||||
nvlist_t *nvl = nvlist_create(0); | nvlist_t *nvl = nvlist_create(0); | ||||
nvlist_add_number(nvl, "type", addr->type); | nvlist_add_number(nvl, "type", addr->type); | ||||
nvlist_add_number(nvl, "iflags", addr->iflags); | nvlist_add_number(nvl, "iflags", addr->iflags); | ||||
if (addr->type == PF_ADDR_DYNIFTL) | |||||
nvlist_add_string(nvl, "ifname", addr->v.ifname); | nvlist_add_string(nvl, "ifname", addr->v.ifname); | ||||
if (addr->type == PF_ADDR_TABLE) | |||||
nvlist_add_string(nvl, "tblname", addr->v.tblname); | nvlist_add_string(nvl, "tblname", addr->v.tblname); | ||||
pfctl_nv_add_addr(nvl, "addr", &addr->v.a.addr); | pfctl_nv_add_addr(nvl, "addr", &addr->v.a.addr); | ||||
pfctl_nv_add_addr(nvl, "mask", &addr->v.a.mask); | pfctl_nv_add_addr(nvl, "mask", &addr->v.a.mask); | ||||
nvlist_add_nvlist(nvparent, name, nvl); | nvlist_add_nvlist(nvparent, name, nvl); | ||||
} | } | ||||
static void | static void | ||||
pf_nvaddr_wrap_to_addr_wrap(const nvlist_t *nvl, struct pf_addr_wrap *addr) | pf_nvaddr_wrap_to_addr_wrap(const nvlist_t *nvl, struct pf_addr_wrap *addr) | ||||
{ | { | ||||
addr->type = nvlist_get_number(nvl, "type"); | addr->type = nvlist_get_number(nvl, "type"); | ||||
addr->iflags = nvlist_get_number(nvl, "iflags"); | addr->iflags = nvlist_get_number(nvl, "iflags"); | ||||
strlcpy(addr->v.ifname, nvlist_get_string(nvl, "ifname"), IFNAMSIZ); | if (addr->type == PF_ADDR_DYNIFTL) | ||||
strlcpy(addr->v.ifname, nvlist_get_string(nvl, "ifname"), | |||||
IFNAMSIZ); | |||||
if (addr->type == PF_ADDR_TABLE) | |||||
donner: Could we have a "switch" here to catch the other (missing) cases?
So those addr->ifname might… | |||||
kpAuthorUnsubmitted Done Inline ActionsI think I'd prefer to leave it as is. The fields under addr->v are part of a union, so they used to just always be copied in. We now do a bit more validation, which is what caused the bug this fixes: we checked the length of the ifname string, but that overlapped (because union) with the tblname string, which can be longer. In fact, it turns out that for type == PF_ADDR_DYNIFTL we still use the mask value, so the resulting conditional would end up being complex (and thus error-prone). kp: I think I'd prefer to leave it as is. The fields under addr->v are part of a union, so they… | |||||
strlcpy(addr->v.tblname, nvlist_get_string(nvl, "tblname"), | strlcpy(addr->v.tblname, nvlist_get_string(nvl, "tblname"), | ||||
PF_TABLE_NAME_SIZE); | PF_TABLE_NAME_SIZE); | ||||
pf_nvaddr_to_addr(nvlist_get_nvlist(nvl, "addr"), &addr->v.a.addr); | pf_nvaddr_to_addr(nvlist_get_nvlist(nvl, "addr"), &addr->v.a.addr); | ||||
pf_nvaddr_to_addr(nvlist_get_nvlist(nvl, "mask"), &addr->v.a.mask); | pf_nvaddr_to_addr(nvlist_get_nvlist(nvl, "mask"), &addr->v.a.mask); | ||||
} | } | ||||
static void | static void | ||||
pfctl_nv_add_rule_addr(nvlist_t *nvparent, const char *name, | pfctl_nv_add_rule_addr(nvlist_t *nvparent, const char *name, | ||||
const struct pf_rule_addr *addr) | const struct pf_rule_addr *addr) | ||||
▲ Show 20 Lines • Show All 431 Lines • Show Last 20 Lines |
Could we have a "switch" here to catch the other (missing) cases?
So those addr->ifname might be unset in contrast to the previous version. Can this cause any harm? What's the default?