Re: CID 1452909: Use of untrusted scalar value (pf_table.c)

[Jonathan Gray]

On Tue, Aug 15, 2017 at 02:40:32PM +0200, Mike Belopuhov wrote:
> Hi,
> 
> Coverity has discovered that we're blindly trusting the value
> of pfra_type that we read from the userland supplied pfr_addr
> and use it to index an array of pools in pfr_create_kentry.
> 
> I suggest to do two things: add a check in pfr_validate_addr
> that is called after every copyin and also perform the check
> in pfr_create_kentry before we attempt to use the value.
> 
> OK?

ok jsg@

CID 1452909: Use of untrusted scalar value (pf_table.c)

[Mike Belopuhov]

Hi,

Coverity has discovered that we're blindly trusting the value
of pfra_type that we read from the userland supplied pfr_addr
and use it to index an array of pools in pfr_create_kentry.

I suggest to do two things: add a check in pfr_validate_addr
that is called after every copyin and also perform the check
in pfr_create_kentry before we attempt to use the value.

OK?

P.S.
What does 'k' table and entry prefix stand for in pf_table.c?
Kernel?

diff --git sys/net/pf_table.c sys/net/pf_table.c
index 7666ec7013c..985c673b5cb 100644
--- sys/net/pf_table.c
+++ sys/net/pf_table.c
@@ -741,10 +741,12 @@ pfr_validate_addr(struct pfr_addr *ad)
return (-1);
if (ad->pfra_not && ad->pfra_not != 1)
return (-1);
if (ad->pfra_fback)
return (-1);
+   if (ad->pfra_type >= PFRKE_MAX)
+   return (-1);
return (0);
 }
 
 void
 pfr_enqueue_addrs(struct pfr_ktable *kt, struct pfr_kentryworkq *workq,
@@ -820,10 +822,13 @@ pfr_lookup_addr(struct pfr_ktable *kt, struct pfr_addr 
*ad, int exact)
 struct pfr_kentry *
 pfr_create_kentry(struct pfr_addr *ad)
 {
struct pfr_kentry_all   *ke;
 
+   if (ad->pfra_type >= PFRKE_MAX)
+   panic("unknown pfra_type %d", ad->pfra_type);
+
ke = pool_get(_kentry_pl[ad->pfra_type], PR_NOWAIT | PR_ZERO);
if (ke == NULL)
return (NULL);
 
ke->pfrke_type = ad->pfra_type;
@@ -842,13 +847,10 @@ pfr_create_kentry(struct pfr_addr *ad)
if (ad->pfra_ifname[0])
ke->pfrke_rkif = pfi_kif_get(ad->pfra_ifname);
if (ke->pfrke_rkif)
pfi_kif_ref(ke->pfrke_rkif, PFI_KIF_REF_ROUTE);
break;
-   default:
-   panic("unknown pfrke_type %d", ke->pfrke_type);
-   break;
}
 
switch (ad->pfra_af) {
case AF_INET:
FILLIN_SIN(ke->pfrke_sa.sin, ad->pfra_ip4addr);