Closed Bug 800502 Opened 7 years ago Closed 7 years ago

NULL deref in ccsip_subsmanager.c:add_content()

Categories

(Core :: WebRTC: Signaling, defect, major)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: jesup, Assigned: ehugg)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

eventBody has to be NULL on the first pass through the loop

I suspect we're never calling this code, currently, so impact is minimal.
Attachment #670509 - Flags: review?(rjesup)
Comment on attachment 670509 [details] [diff] [review]
Signaling -  protect strlen from NULL input in add_content

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

::: media/webrtc/signaling/src/sipcc/core/sipstack/ccsip_subsmanager.c
@@ +1511,5 @@
>  
>          switch (eventData->type) {
>          case EVENT_DATA_RAW:
>              // Assume body is of type CMXML for now
>              (void) sippmh_add_message_body(request, eventBody, len,

Still wrong, as this assumes eventBody isn't null.  Probably should be some type of buffer or allocation.
Attachment #670509 - Flags: review?(rjesup) → review-
Comment on attachment 670509 [details] [diff] [review]
Signaling -  protect strlen from NULL input in add_content


You're right this function is broken.  Checking original as well as latest version.
Attachment #670509 - Attachment is obsolete: true
Broken during removal of libxml2 dependency  Original encodes the eventData in XML.

    while (eventData) {
        eventBody = xmlEncodeEventData(eventData);
        if (!eventBody) {
            CCSIP_DEBUG_ERROR(SIP_F_PREFIX"%s: Framing XML Body failed\n", fname1, fname);
            return (FALSE);
        }
        len = strlen(eventBody);

I'm now considering #if 0 of the function body and returning FALSE.
Comment on attachment 670530 [details] [diff] [review]
Signaling - Removed unused code that required XML handling


This function cannot be repaired without XML handling.  It is used in sipSPISendSubscribe, sipSPISendSubNotify, and sipSPISendPublish.  We could do something like this, or remove more of Sub/Pub/Notify.
Attachment #670530 - Flags: review?(rjesup)
Attachment #670530 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/7cb0e7444c55

Should this have a test?
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
No testsuite needed; the code has no way currently to call it (part of the SIP functionality not currently hooked up), and the solution to the bug caused by removing the XML config code was to stub this function out.
Flags: in-testsuite? → in-testsuite-
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.