On Mon, 2023-03-20 at 12:52 -0700, Kees Cook wrote: > > What sort of patch would you like here? How should the other cases in > the switch statement behave? > > Are these the correct bounds? > > WLAN_CIPHER_SUITE_CCMP: keylen <= 16 > WLAN_CIPHER_SUITE_TKIP: keylen <= 16 > WLAN_CIPHER_SUITE_WEP104: keylen <= 13 > WLAN_CIPHER_SUITE_WEP40: keylen <= 13 40 bits is only 5 bytes :-) > and should it silently ignore larger values in each case? > For the cases other than TKIP, no changes should be necessary - in those cases, the key will always be == the value from the (corrected) table above. For TKIP, the keylen will be 32, but comprised of the actual encryption key and then MIC keys, so only 16 bytes should be relevant. I don't really care how you handle this. We can be bug-compatible with the old code, then I'd say from your patch only the changes in the TKIP case are needed. Or we can just limit the copy to only 16 bytes, but that would need some validation that it still works. From memory, I'd say it will still work, and I'd even say the whole memcpy() might not be needed in the TKIP case at all since the iwldvm firmware deals with phase 1 keys (P1K) to derive phase 2 keys, not the original encryption key. So simpler is probably to just take the TKIP hunk from your patch, even knowing that the memcpy() there is almost certainly incorrect now. johannes
