* [PATCH v2] netfilter: nf_tables: Check for overflow of u8 fields from u32 netlink attributes
@ 2016-08-14 14:59 Laura Garcia Liebana
2016-08-17 15:53 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Laura Garcia Liebana @ 2016-08-14 14:59 UTC (permalink / raw)
To: netfilter-devel
Fix the direct assignment from u32 data input into an attribute with a
size of u8.
Refer to 4da449ae1df
Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
---
Changes in V2:
- Collapse the 5 independent patches in just one
- Change description and subject
- Add bug link
net/netfilter/nft_bitwise.c | 7 ++++++-
net/netfilter/nft_byteorder.c | 13 +++++++++++--
net/netfilter/nft_cmp.c | 5 ++++-
net/netfilter/nft_immediate.c | 3 +++
net/netfilter/nft_nat.c | 2 ++
5 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c
index d71cc18..2c49f69 100644
--- a/net/netfilter/nft_bitwise.c
+++ b/net/netfilter/nft_bitwise.c
@@ -53,6 +53,7 @@ static int nft_bitwise_init(const struct nft_ctx *ctx,
struct nft_bitwise *priv = nft_expr_priv(expr);
struct nft_data_desc d1, d2;
int err;
+ u32 len;
if (tb[NFTA_BITWISE_SREG] == NULL ||
tb[NFTA_BITWISE_DREG] == NULL ||
@@ -61,7 +62,11 @@ static int nft_bitwise_init(const struct nft_ctx *ctx,
tb[NFTA_BITWISE_XOR] == NULL)
return -EINVAL;
- priv->len = ntohl(nla_get_be32(tb[NFTA_BITWISE_LEN]));
+ len = ntohl(nla_get_be32(tb[NFTA_BITWISE_LEN]));
+ if (len > U8_MAX)
+ return -EINVAL;
+ priv->len = len;
+
priv->sreg = nft_parse_register(tb[NFTA_BITWISE_SREG]);
err = nft_validate_register_load(priv->sreg, priv->len);
if (err < 0)
diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
index b78c28b..fdd23d5 100644
--- a/net/netfilter/nft_byteorder.c
+++ b/net/netfilter/nft_byteorder.c
@@ -100,6 +100,7 @@ static int nft_byteorder_init(const struct nft_ctx *ctx,
{
struct nft_byteorder *priv = nft_expr_priv(expr);
int err;
+ u32 len, size;
if (tb[NFTA_BYTEORDER_SREG] == NULL ||
tb[NFTA_BYTEORDER_DREG] == NULL ||
@@ -117,7 +118,10 @@ static int nft_byteorder_init(const struct nft_ctx *ctx,
return -EINVAL;
}
- priv->size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE]));
+ size = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_SIZE]));
+ if (size > U8_MAX)
+ return -EINVAL;
+ priv->size = size;
switch (priv->size) {
case 2:
case 4:
@@ -128,7 +132,12 @@ static int nft_byteorder_init(const struct nft_ctx *ctx,
}
priv->sreg = nft_parse_register(tb[NFTA_BYTEORDER_SREG]);
- priv->len = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_LEN]));
+
+ len = ntohl(nla_get_be32(tb[NFTA_BYTEORDER_LEN]));
+ if (len > U8_MAX)
+ return -EINVAL;
+ priv->len = len;
+
err = nft_validate_register_load(priv->sreg, priv->len);
if (err < 0)
return err;
diff --git a/net/netfilter/nft_cmp.c b/net/netfilter/nft_cmp.c
index e25b35d..ca247e5 100644
--- a/net/netfilter/nft_cmp.c
+++ b/net/netfilter/nft_cmp.c
@@ -84,8 +84,11 @@ static int nft_cmp_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
if (err < 0)
return err;
- priv->op = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
+ if (desc.len > U8_MAX)
+ return -EINVAL;
priv->len = desc.len;
+ priv->op = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
+
return 0;
}
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index db3b746..6de590c 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -53,6 +53,9 @@ static int nft_immediate_init(const struct nft_ctx *ctx,
tb[NFTA_IMMEDIATE_DATA]);
if (err < 0)
return err;
+
+ if (desc.len > U8_MAX)
+ return -EINVAL;
priv->dlen = desc.len;
priv->dreg = nft_parse_register(tb[NFTA_IMMEDIATE_DREG]);
diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c
index ee2d717..74f8293 100644
--- a/net/netfilter/nft_nat.c
+++ b/net/netfilter/nft_nat.c
@@ -148,6 +148,8 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
family = ntohl(nla_get_be32(tb[NFTA_NAT_FAMILY]));
if (family != ctx->afi->family)
return -EOPNOTSUPP;
+ if (family > U8_MAX)
+ return -EINVAL;
switch (family) {
case NFPROTO_IPV4:
--
2.8.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] netfilter: nf_tables: Check for overflow of u8 fields from u32 netlink attributes
2016-08-14 14:59 [PATCH v2] netfilter: nf_tables: Check for overflow of u8 fields from u32 netlink attributes Laura Garcia Liebana
@ 2016-08-17 15:53 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-17 15:53 UTC (permalink / raw)
To: Laura Garcia Liebana; +Cc: netfilter-devel
On Sun, Aug 14, 2016 at 04:59:36PM +0200, Laura Garcia Liebana wrote:
> diff --git a/net/netfilter/nft_cmp.c b/net/netfilter/nft_cmp.c
> index e25b35d..ca247e5 100644
> --- a/net/netfilter/nft_cmp.c
> +++ b/net/netfilter/nft_cmp.c
> @@ -84,8 +84,11 @@ static int nft_cmp_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
> if (err < 0)
> return err;
>
> - priv->op = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
> + if (desc.len > U8_MAX)
> + return -EINVAL;
I suggest we use return -ERANGE instead of -EINVAL.
> priv->len = desc.len;
> + priv->op = ntohl(nla_get_be32(tb[NFTA_CMP_OP]));
Hm, I just noticed this priv->op is not validated either?
If so, we should add NFT_CMP_MAX to enum nft_cmp_ops and check if this
is:
if (priv->op >= NFT_CMP_MAX)
return -ERANGE;
You can send this in a follow up patch given that this doesn't match
the description, or you rework the title and description to make this
all fit in one logical change.
> diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
> index db3b746..6de590c 100644
> --- a/net/netfilter/nft_immediate.c
> +++ b/net/netfilter/nft_immediate.c
> @@ -53,6 +53,9 @@ static int nft_immediate_init(const struct nft_ctx *ctx,
> tb[NFTA_IMMEDIATE_DATA]);
> if (err < 0)
> return err;
> +
> + if (desc.len > U8_MAX)
> + return -EINVAL;
> priv->dlen = desc.len;
>
> priv->dreg = nft_parse_register(tb[NFTA_IMMEDIATE_DREG]);
> diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c
> index ee2d717..74f8293 100644
> --- a/net/netfilter/nft_nat.c
> +++ b/net/netfilter/nft_nat.c
> @@ -148,6 +148,8 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
> family = ntohl(nla_get_be32(tb[NFTA_NAT_FAMILY]));
> if (family != ctx->afi->family)
> return -EOPNOTSUPP;
> + if (family > U8_MAX)
> + return -EINVAL;
I think we don't need this, because we check later on:
switch (family) {
case NFPROTO_IPV4:
...
case NFPROTO_IPV6:
...
default:
return -EAFNOSUPPORT;
}
So this check is superfluous, you can remove it.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-08-17 16:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-14 14:59 [PATCH v2] netfilter: nf_tables: Check for overflow of u8 fields from u32 netlink attributes Laura Garcia Liebana
2016-08-17 15:53 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).