lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 27 Apr 2019 11:50:41 +0200
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     netdev@...r.kernel.org, Johannes Berg <johannes.berg@...el.com>
Subject: Re: [PATCH] netlink: limit recursion depth in policy validation

On Fri, Apr 26, 2019 at 02:13:46PM +0200, Johannes Berg wrote:
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 4fc7c122e916..09a17b30ba73 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -219,6 +219,8 @@ static int validate_ie_attr(const struct nlattr *attr,
>  }
>  
>  /* policy for the attributes */
> +static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR];
> +
>  static const struct nla_policy
>  nl80211_ftm_responder_policy[NL80211_FTM_RESP_ATTR_MAX + 1] = {
>  	[0] = { .strict_start_type = NL80211_FTM_RESP_ATTR_CIVICLOC + 1 },
> @@ -268,11 +270,7 @@ static const struct nla_policy
>  nl80211_psmr_peer_attr_policy[NL80211_PMSR_PEER_ATTR_MAX + 1] = {
>  	[0] = { .strict_start_type = NL80211_PMSR_PEER_ATTR_RESP + 1 },
>  	[NL80211_PMSR_PEER_ATTR_ADDR] = NLA_POLICY_ETH_ADDR,
> -	/*
> -	 * we could specify this again to be the top-level policy,
> -	 * but that would open us up to recursion problems ...
> -	 */
> -	[NL80211_PMSR_PEER_ATTR_CHAN] = { .type = NLA_NESTED },
> +	[NL80211_PMSR_PEER_ATTR_CHAN] = NLA_POLICY_NESTED(nl80211_policy),

I guess you only allow one more nested instance of this attribute?

I mean, how many times is NL80211 allow to recurse on this?

Probably you can define a new nl80211_policy_recurse object and set a
flag somewhere to describe that no more recursion are permitted?

I would try to handle this from nl80211, instead of from the core by
limiting recursions to 10.

Once we expose the descriptions to userspace, I would expect we'll end
with tools to validate all kind of stuff like this, eg. fuzzy tested,
check for recursions like this (which IMO they should not be allowed).

Powered by blists - more mailing lists