[PATCH 1/3] respect limitedness of the phy name buffer

[Leon Busch-George]

Oops - sorry! Typos, missing words, missing subject prefix.. Let me resubmit that.

On Sat,  2 Mar 2024 00:16:20 +0100
"Leon M. Busch-George" <leon at georgemail.de> wrote:

> From: "Leon M. Busch-George" <leon at georgemail.eu>
> 
> This prevents potential buffer overflows while writing to the phy
> name buffer buffer. Additionally, truncated data is not returned so
> consumers don't work with unterminated data, preventing out-of-bounds
> access.
> 
> Sadly, consumers like lookup_phy or phyname don't the size of their
> respective target buffers without changing the interface.
> 
> Signed-off-by: Leon M. Busch-George <leon at georgemail.eu>
> ---
>  iwinfo_nl80211.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
> index 2200249..2ea5925 100644
> --- a/iwinfo_nl80211.c
> +++ b/iwinfo_nl80211.c
> @@ -34,6 +34,7 @@
>  #define min(x, y) ((x) < (y)) ? (x) : (y)
>  
>  #define BIT(x) (1ULL<<(x))
> +#define PHY_NAME_BUFFER_SIZE (32)
>  
>  static struct nl80211_state *nls = NULL;
>  
> @@ -761,31 +762,36 @@ static int nl80211_phyname_cb(struct nl_msg
> *msg, void *arg) char *buf = arg;
>  	struct nlattr **attr = nl80211_parse(msg);
>  
> -	if (attr[NL80211_ATTR_WIPHY_NAME])
> -		memcpy(buf, nla_data(attr[NL80211_ATTR_WIPHY_NAME]),
> -		       nla_len(attr[NL80211_ATTR_WIPHY_NAME]));
> -	else
> +	if (!attr[NL80211_ATTR_WIPHY_NAME]) {
>  		buf[0] = 0;
> +		return NL_SKIP;
> +	}
> +
> +	int len = nla_len(attr[NL80211_ATTR_WIPHY_NAME]);
> +	if (len > PHY_NAME_BUFFER_SIZE)
> +		len = PHY_NAME_BUFFER_SIZE;
> +
> +	memcpy(buf, nla_data(attr[NL80211_ATTR_WIPHY_NAME]), len);
>  
>  	return NL_SKIP;
>  }
>  
>  static char * nl80211_ifname2phy(const char *ifname)
>  {
> -	static char phy[32] = { 0 };
> +	static char phy[PHY_NAME_BUFFER_SIZE] = { 0 };
>  
>  	memset(phy, 0, sizeof(phy));
>  
>  	nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
>  	                nl80211_phyname_cb, phy);
>  
> -	return phy[0] ? phy : NULL;
> +	return (phy[0] && !phy[sizeof(phy) - 1]) ? phy : NULL;
>  }
>  
>  static char * nl80211_phyidx2name(unsigned int idx)
>  {
>  	struct nl80211_msg_conveyor *cv;
> -	static char phy[32] = { 0 };
> +	static char phy[PHY_NAME_BUFFER_SIZE] = { 0 };
>  
>  	if (nl80211_init() < 0)
>  		return NULL;
> @@ -799,7 +805,7 @@ static char * nl80211_phyidx2name(unsigned int
> idx) memset(phy, 0, sizeof(phy));
>  	nl80211_send(cv, nl80211_phyname_cb, phy);
>  
> -	return phy[0] ? phy : NULL;
> +	return (phy[0] && !phy[sizeof(phy) - 1]) ? phy : NULL;
>  
>  nla_put_failure:
>  	return NULL;