Closed Bug 800502 Opened 9 years ago Closed 9 years ago

NULL deref in ccsip_subsmanager.c:add_content()


(Core :: WebRTC: Signaling, defect)

Not set





(Reporter: jesup, Assigned: ehugg)



(Whiteboard: [qa-])


(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+

Should this have a test?
Closed: 9 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.