diff mbox series

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

Message ID 20240301231721.139669-2-leon@georgemail.de
State New
Headers show
Series [1/3] respect limitedness of the phy name buffer | expand

Commit Message

Leon M. Busch-George March 1, 2024, 11:16 p.m. UTC 
From: "Leon M. Busch-George" <leon@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@georgemail.eu>
---
 iwinfo_nl80211.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Leon Busch-George March 2, 2024, 10:49 a.m. UTC | #1 
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@georgemail.de> wrote:

> From: "Leon M. Busch-George" <leon@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@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;
diff mbox series

Patch

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;