| Message ID | 20231116154439.33880-2-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dpp: check that DPP is running in station watch | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
Hi James, On 11/16/23 09:44, James Prestwood wrote: > In theory this shouldn't be possible because the configuration object > validates that the SSID is utf-8. But it doesn't hurt to check > especially since we can't control what the kernel sends us. > --- > src/dpp.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/dpp.c b/src/dpp.c > index 18b2a7c6..8da79603 100644 > --- a/src/dpp.c > +++ b/src/dpp.c > @@ -884,6 +884,9 @@ static bool dpp_scan_results(int err, struct l_queue *bss_list, > /* Purely for grabbing the SSID */ > bss = l_queue_peek_head(bss_list); > > + if (L_WARN_ON(!util_ssid_is_utf8(bss->ssid_len, bss->ssid))) > + goto reset; > + This still seems brittle. You have the validated SSID from the DPP session, shouldn't you be storing that and using it to filter the scan results? There's no guarantee that a filtered active scan is going to return only the SSID you asked for (lets say a misbehaving or malicious AP), so assuming that the first BSS in the scan results list is the SSID you want isn't really guaranteed. > memcpy(ssid, bss->ssid, bss->ssid_len); > ssid[bss->ssid_len] = '\0'; > Regards, -Denis
Hi Denis, On 11/16/23 07:51, Denis Kenzior wrote: > Hi James, > > On 11/16/23 09:44, James Prestwood wrote: >> In theory this shouldn't be possible because the configuration object >> validates that the SSID is utf-8. But it doesn't hurt to check >> especially since we can't control what the kernel sends us. >> --- >> src/dpp.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/src/dpp.c b/src/dpp.c >> index 18b2a7c6..8da79603 100644 >> --- a/src/dpp.c >> +++ b/src/dpp.c >> @@ -884,6 +884,9 @@ static bool dpp_scan_results(int err, struct >> l_queue *bss_list, >> /* Purely for grabbing the SSID */ >> bss = l_queue_peek_head(bss_list); >> + if (L_WARN_ON(!util_ssid_is_utf8(bss->ssid_len, bss->ssid))) >> + goto reset; >> + > > This still seems brittle. You have the validated SSID from the DPP > session, shouldn't you be storing that and using it to filter the scan > results? There's no guarantee that a filtered active scan is going to > return only the SSID you asked for (lets say a misbehaving or > malicious AP), so assuming that the first BSS in the scan results list > is the SSID you want isn't really guaranteed. Sure I can do that. I was banking on the kernel filtering, but we can be 100% sure and just store the SSID in the dpp_sm. > >> memcpy(ssid, bss->ssid, bss->ssid_len); >> ssid[bss->ssid_len] = '\0'; > > Regards, > -Denis
diff --git a/src/dpp.c b/src/dpp.c index 18b2a7c6..8da79603 100644 --- a/src/dpp.c +++ b/src/dpp.c @@ -884,6 +884,9 @@ static bool dpp_scan_results(int err, struct l_queue *bss_list, /* Purely for grabbing the SSID */ bss = l_queue_peek_head(bss_list); + if (L_WARN_ON(!util_ssid_is_utf8(bss->ssid_len, bss->ssid))) + goto reset; + memcpy(ssid, bss->ssid, bss->ssid_len); ssid[bss->ssid_len] = '\0';