Closed Bug 505277 Opened 11 years ago Closed 11 years ago

strncpy without null termination in nsWifiScannerUnix

Categories

(Core :: DOM: Geolocation, defect)

All
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1
Tracking Status
status1.9.1 --- .3-fixed

People

(Reporter: dbaron, Assigned: dougt)

Details

(Keywords: verified1.9.1, Whiteboard: [sg:nse])

Attachments

(1 file)

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.)
Attached patch patch v.1Splinter Review
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+
http://hg.mozilla.org/mozilla-central/rev/02913e3a7eaf
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #389834 - Flags: approval1.9.1.2?
(btw, asked bc on irc.mozilla.org #qa tested last night)
s/tested/to test
Attachment #389834 - Flags: approval1.9.1.2? → approval1.9.1.3?
Comment on attachment 389834 [details] [diff] [review]
patch v.1

Not for 1.9.1.2.
Comment on attachment 389834 [details] [diff] [review]
patch v.1

Approved for 1.9.1.3. a=ss
Attachment #389834 - Flags: approval1.9.1.3? → approval1.9.1.3+
Doug: Can you get this landed for 1.9.1.3? Code freeze is tonight...
Verified fixed based on check-ins into trunk and 1.9.1 branch.
Status: RESOLVED → VERIFIED
Keywords: verified1.9.1
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?
Whiteboard: [sg:nse]
Group: core-security
You need to log in before you can comment on or make changes to this bug.