Closed Bug 505277 Opened 11 years ago Closed 11 years ago
strncpy without null termination in ns
Wifi Scanner Unix
nsWifiScannerUnix calls strncpy when setting ap->mSsid without null-terminating the buffer. This means that if |buffer| contains a string longer than 31 characters, ap->mSsid will be missing its null-terminator: strncpy(ap->mSsid, buffer, 32); and anything that reads from ap->mSsid will get arbitrary data past the end of the string out of memory. Instead, this code should probably use the SetSsid method that other wifi scanner implementations use, which handles the null-termination correctly. (Likewise, for good code hygiene, the code below that assigns directly to ap->mSignal should probably use ap->SetSignal() instead.)
untested -- my vm doesn't have wifi hardware exposed. i can give it a go tomorrow.
Assignee: nobody → doug.turner
Attachment #389834 - Flags: review?
Attachment #389834 - Flags: review? → review?(dbaron)
Attachment #389834 - Flags: review?(dbaron) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(btw, asked bc on irc.mozilla.org #qa tested last night)
Attachment #389834 - Flags: approval184.108.40.206? → approval220.127.116.11?
Comment on attachment 389834 [details] [diff] [review] patch v.1 Approved for 18.104.22.168. a=ss
Attachment #389834 - Flags: approval22.214.171.124? → approval126.96.36.199+
Doug: Can you get this landed for 188.8.131.52? Code freeze is tonight...
Verified fixed based on check-ins into trunk and 1.9.1 branch.
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.2a1
Not sure this is much of a security problem, maybe we're leaking a bit of memory to someone? Is some part of our app using the data assuming it's sanitized or not too long and causing problems elsewhere?
You need to log in before you can comment on or make changes to this bug.