Closed Bug 1229726 Opened 4 years ago Closed 4 years ago

[MDNS] fix conversion from sockaddr to NetAddr

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
blocking-b2g 2.5+
Tracking Status
firefox46 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: schien, Assigned: schien)

References

Details

Attachments

(1 file, 2 obsolete files)

struct sockaddr has extra "sa_len" field on OSX and FreeBSD, we cannot memcpy a sockaddr to Netaddr.
https://dxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderReply.cpp#284
copy sockaddr.sa_family and sockaddr.sa_data separately instead of doing memcpy on sockaddr.
Attachment #8695686 - Flags: feedback?(mcmanus)
Comment on attachment 8695686 [details] [diff] [review]
bug1229726-fix-for-sockaddr-sa_len.patch

Review of attachment 8695686 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/dns/mdns/libmdns/MDNSResponderReply.cpp
@@ +281,5 @@
>    }
>  
>    NetAddr address;
> +  address.raw.family = aAddress->sa_family;
> +  memcpy(&address.raw.data, aAddress->sa_data, sizeof(aAddress->sa_data));

I don't like that sizeof checking the size of the src only - seems like it could end up with an overwrite. probably need to make sure dest is >= size of src. otherwise lgtm
Attachment #8695686 - Flags: feedback?(mcmanus) → feedback+
Use static assert to check the size of dest.
Attachment #8695686 - Attachment is obsolete: true
Attachment #8701294 - Flags: review?(mcmanus)
Comment on attachment 8701294 [details] [diff] [review]
bug1229726-fix-for-sockaddr-sa_len.patch

sorry, wrong patch file.
Attachment #8701294 - Flags: review?(mcmanus)
Attachment #8701294 - Attachment is obsolete: true
Attachment #8701353 - Flags: review?(mcmanus) → review+
Hi SC,
POlease raise 2.5 uplift request. Thanks!
Flags: needinfo?(schien)
blocking-b2g: --- → 2.5+
Comment on attachment 8701353 [details]
MozReview Request: Bug 1229726 - fix the data copy from sockaddr to NetAddr on OSX/FreeBSD. r=mcmanus.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 1200132
User impact if declined: User cannot launch presentation request from OS X and FreeBSD. No impact for device that receiving presentation request but buffer copy issue might be potential security hole.
Testing completed: done by manual test.
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: N/A
Attachment #8701353 - Flags: approval‑mozilla‑b2g44?
Flags: needinfo?(schien)
Comment on attachment 8701353 [details]
MozReview Request: Bug 1229726 - fix the data copy from sockaddr to NetAddr on OSX/FreeBSD. r=mcmanus.

Approved for Tv2.5
Attachment #8701353 - Flags: approval‑mozilla‑b2g44? → approval‑mozilla‑b2g44+
https://hg.mozilla.org/mozilla-central/rev/8828d3da388f
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.