[SR-Users] Does xcap server of kamailio support partial update of xcap document?

laura testi lau.testi at gmail.com
Tue Sep 13 17:33:25 CEST 2011


Hi Daniel,
we found further bugs in the file xcap_server.c (lastest version on
master branch with the patches).
The main problems are following:
-  xcap server insert the xml doc into DB without any validation of
the xml doc for the operation
   PUT, which can cause core dump when the DELETE operation is
performed if the XML doc
   is invalid in DB
- the kamailio crash when DELETE operation is performed if in the DB
there is no referenced
  xml doc


We have done further patches on the lastest xcap_server.c from the
master branch.

Here are the diff:

# diff -u  xcap_server.c.orig  xcap_server.c.new
--- xcap_server.c.orig           2011-09-13 11:52:19.000000000 +0200
+++ xcap_server.c.new       2011-09-13 15:12:54.000000000 +0200
@@ -30,6 +30,8 @@
#include <unistd.h>
#include <fcntl.h>
#include <time.h>
+#include <libxml/xpath.h>
+#include <libxml/xpathInternals.h>

#include "../../lib/srdb1/db.h"
#include "../../pt.h"
@@ -334,7 +336,14 @@
        db_key_t qcols[9];
        db_val_t qvals[9];
        int ncols = 0;
-
+       xmlDocPtr docxml = NULL;
+
+       docxml = xmlParseMemory(doc->s, doc->len);
+       if(docxml == NULL)
+       {
+               LM_ERR("Error inserting Invalid Xml Document into database \n");
+               goto error;
+       }

        /* insert in xcap table*/
        qcols[ncols] = &str_username_col;
@@ -400,6 +409,7 @@

        return 0;
error:
+if(docxml!=NULL) xmlFreeDoc(docxml);
        return -1;
}

@@ -603,6 +613,7 @@
        int nrcols = 0;
        db1_res_t* db_res = NULL;
        str s;
+       xmlDocPtr docxml = NULL;

        /* returned cols from table xcap*/
        rcols[nrcols] = &str_doc_col;
@@ -680,7 +691,15 @@
        memcpy(doc->s, s.s, s.len);
        doc->s[doc->len] = '\0';

+       docxml = xmlParseMemory(doc->s, doc->len);
+       if(docxml == NULL)
+       {
+               LM_ERR("Invalid XmlDocument in database \n");
+               goto error;
+       }
+
        xcaps_dbf.free_result(xcaps_db, db_res);
+       if(docxml!=NULL) xmlFreeDoc(docxml);
        return 0;

notfound:
@@ -690,6 +709,7 @@
error:
        if(db_res!=NULL)
                xcaps_dbf.free_result(xcaps_db, db_res);
+       if(docxml!=NULL) xmlFreeDoc(docxml);
        return -1;
}

@@ -970,7 +990,7 @@
        str uri;
        str path;
        xcap_uri_t xuri;
-       str body;
+       str body = {0, 0};
        str etag_hdr;
        str etag;
        str tbuf;
@@ -1041,7 +1061,7 @@
                                &xcaps_str_empty, &xcaps_str_empty);
        } else {
                /* delete element */
-               if(xcaps_get_db_doc(&turi.user, &turi.host, &xuri, &tbuf)<0)
+               if(xcaps_get_db_doc(&turi.user, &turi.host, &xuri, &tbuf)!=0)
                {
                        LM_ERR("could not fetch xcap document\n");
                        goto error;


Please find the attached xcap_server.c.new

Best Regards,
Laura


On Thu, Sep 8, 2011 at 4:08 PM, Daniel-Constantin Mierla
<miconda at gmail.com> wrote:
> Hello Laura,
>
> thanks for testing and further patching. I just committed on master branch.
> Are you working with 3.1? Just to see how fast to plan the backports.
>
> Cheers,
> Daniel
>
> On 9/8/11 3:22 PM, laura testi wrote:
>
> Hi,
> we have tested the patch, it does not work. We have done some minor
> changes. Now it works.
>
> Here are the diff:
>
> diff -u xcap_misc.c  xcap_misc.c_patch
> --- xcap_misc.c       2011-09-08 14:23:37.000000000 +0200
> +++ xcap_misc.c_patch 2011-09-08 14:29:24.000000000 +0200
> @@ -465,7 +465,7 @@
>                 goto error;
>         }
>      nodes = xpathObj->nodesetval;
> -       if(nodes==NULL)
> +       if(nodes==NULL || nodes->nodeNr==0 || nodes->nodeTab == NULL)
>         {
>                 /* no selection for xpath expression */
>                 LM_DBG("no selection for xpath expression [%s]\n",
> xpaths->s);
> @@ -488,7 +488,7 @@
>                 }
>                 *p = '/';
>                 nodes = xpathObj->nodesetval;
> -               if(nodes==NULL)
> +               if(nodes==NULL || nodes->nodeNr==0 || nodes->nodeTab ==
> NULL)
>                 {
>                         LM_DBG("no selection for xpath parent
> expression [%s]\n",
>                                         xpaths->s);
>
>
>
> Please see the patch file in the attachment.
>
> Thanks  again
>
> Best Regards,
> Laura
>
> On Wed, Sep 7, 2011 at 10:12 PM, Daniel-Constantin Mierla
> <miconda at gmail.com> wrote:
>
> Hello,
>
> inserting a new node seems not to be possible through xpath operations only,
> so I made a patch to be able to add new nodes (entries) -- see it attached
> -- it is for devel version (master branch), hopefully it applies clean to
> 3.1 if you are using that version.
>
> There was no option to test it at all, I just made sure it compiles. Let me
> know the results, if it runs but fails, send me the output with debug=4. If
> all is ok, I will commit to git repository.
>
> Cheers,
> Daniel
>
> On 9/7/11 3:21 PM, laura testi wrote:
>
> Hi,
> we have patch the xcap_misc.h with "#define XCAP_MAX_URI_SIZE   255"
> which was 127, and extended the length of field doc_uri in the xcap
> table to 256 from 128. Now the errror is gone.
>
> After that we have done some test with curl, following are the results:
> - DELETE an Entry from the list: OK
>   i.e.: curl  -X DELETE
>
> http://<ip>:5060/xcap-root/resource-lists/users/sip:user at domain/index/~~/resource-lists/list/entry%5b at uri=%22sip:u1 at d1%22%5d
>  after this command, the contact u1 at d1 is removed from the contact list.
>
> - UPDATE an Entry from the list: OK
>  i.e.,
> curl -T entry.xml -X PUT
>
> http://<ip>:5060/xcap-root/resource-lists/users/sip:user at domain/index/~~/resource-lists/list%5b at name=%22RootGroup%22%5d/entry%5b at uri=%22sip:u2 at d2%22%5d
>
> if the sip:u2 at d2 entry exists, the entry is replaced with the entry in
> the entry.xml (<entry
> uri="sip:u3 at d3"><display-name>u3</display-name></entry>), which is
> sip:u3 at d3
>
> if the sip:u2 at d2 entry is not exists, the entry of u3 at d3 is not added
> to the list!!! follow the rfc4825, it should be added to the list if
> the entry does not exits.
>
> - Add an Entry from the list: KO
>  i.e.,
> curl -T entry.xml -X PUT
>
> http://<ip>:5060/xcap-root/resource-lists/users/sip:user at domain/index/~~/resource-lists/list%5b at name=%22RootGroup%22%5d/entry%5b at uri=%22sip:u2 at d2%22%5d
>
>  the entry.xml is (<entry
> uri="sip:u2 at d2"><display-name>u2</display-name></entry>)
>
>  the entry sip:u2 at d2 was not in the list before the command and is not
> added after the command.
>
> I think it's a bug of xcap server.
>
>
> Many thanks!
>
> Laura
>
>
>
>
> On Wed, Sep 7, 2011 at 12:17 PM, Juha Heinanen<jh at tutpro.com>  wrote:
>
> Daniel-Constantin Mierla writes:
>
> Feel free to do it, however I think it has to be reviewed where is used,
> though -- it should be safe even size in db is longer as long as the
> insert is done by kamailio itself, not sure we have affected cases when
> the insert is done by external apps, so better double check...
>
> every module that reads data from db should check size of received
> string so that it is not longer than space reserved for it in data
> structures. i hope that no module assumes that received data cannot be
> bigger than what db schema says.
>
> -- juha
>
> _______________________________________________
> SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
> sr-users at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
>
> --
> Daniel-Constantin Mierla -- http://www.asipto.com
> Kamailio Advanced Training, Oct 10-13, Berlin: http://asipto.com/u/kat
> http://linkedin.com/in/miconda -- http://twitter.com/miconda
>
>>
>
> _______________________________________________
> SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
> sr-users at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
>
> --
> Daniel-Constantin Mierla -- http://www.asipto.com
> Kamailio Advanced Training, Oct 10-13, Berlin: http://asipto.com/u/kat
> http://linkedin.com/in/miconda -- http://twitter.com/miconda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: xcap_server.c.new
Type: application/octet-stream
Size: 29113 bytes
Desc: not available
URL: <http://lists.sip-router.org/pipermail/sr-users/attachments/20110913/0b28e52d/attachment-0001.obj>


More information about the sr-users mailing list