dhcpcd: size the escaped-SSID buffers for the worst case - #727
Conversation
print_string() renders each non-printable byte as \NNN - four characters plus terminating NUL, failing with ENOBUFS if there is no room for it. An SSID is up to IF_SSIDLEN (32) bytes, so an escaped SSID needs up to (IF_SSIDLEN * 4) + 1 = 129 bytes. dhcpcd_selectprofile() uses PROFILE_LEN, which is 64 causing: dhcpcd_selectprofile: No buffer space available in the log, the resulting call to read_config() then gets an an empty SSID, so no `profile ssid ...` block matches. dhcpcd_reportssid() uses IF_SSIDLEN * 4, which is 128 and correct except for the NUL, so it fails only on a 32-byte SSID whose every byte escapes. It then logs an error instead of the "connected to Access Point" line. Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe change increases both escaped SSID buffers by one byte. This provides space for the terminating NUL byte in ChangesSSID buffer sizing
Priority: ➖ Normal — Impact reflects medium issue severity. Estimated code review effort: 1 (Trivial) | ~2 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Escaped maximum-length SSIDs can now be matched and reported without running out of buffer space. The targeted sizing correction is ready to merge. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ColinMcInnes
left a comment
There was a problem hiding this comment.
I agree, no security hole here, just a truncated log, and this should resolve that.
print_string() renders each non-printable byte as \NNN - four characters plus terminating NUL, failing with ENOBUFS if there is no room for it. An SSID is up to IF_SSIDLEN (32) bytes, so an escaped SSID needs up to (IF_SSIDLEN * 4) + 1 = 129 bytes.
dhcpcd_selectprofile() uses PROFILE_LEN, which is 64 causing:
dhcpcd_selectprofile: No buffer space available
in the log, the resulting call to read_config() then gets an an empty SSID, so no
profile ssid ...block matches.dhcpcd_reportssid() uses IF_SSIDLEN * 4, which is 128 and correct except for the NUL, so it fails only on a 32-byte SSID whose every byte escapes. It then logs an error instead of the "connected to Access Point" line.
Closes #726