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