[OpenSIPS-Devel] [opensips] modules/db_sqlite: error loading drouting (#471)

[nixon]

(I've never used opensips, and I haven't done any C coding in a decade.)

It seems to me that `new_db_id` takes a `str*` and assigns it to the `char* url` member of a `db_id` struct.  However, the `str*` relies on its `len` member, but the `url` member assumes a null-terminated string.  When the `db_id->url` is later passed to `sqlite3_open`, there's no guarantee that the string was null-terminated.

Is something like the following patch needed to ensure that `db_id` is always null-terminated for `sqlite3_open`?

```
diff --git a/db/db_id.c b/db/db_id.c
index 9efc0eb..c470324 100644
--- a/db/db_id.c
+++ b/db/db_id.c
@@ -240,7 +240,9 @@ struct db_id* new_db_id(const str* url)
        }

        /* store the original url */
-       ptr->url = url->s;
+       ptr->url = pkg_malloc(url->len+1);
+       strncpy(ptr->url, url->s, url->len);
+       ptr->url[url->len] = '\0';

        return ptr;

@@ -291,5 +293,6 @@ void free_db_id(struct db_id* id)
        if (id->password) pkg_free(id->password);
        if (id->host) pkg_free(id->host);
        if (id->database) pkg_free(id->database);
+       if (id->url) pkg_free(id->url);
        pkg_free(id);
 }
```

I butchered up `main.c` to create a simple testcase.  Without the above patch, the `assert` fails.  With the patch, the `id->url` returned from `new_db_id` is what I would expect.

```
diff --git a/main.c b/main.c
index c44ebf2..4ebe8a0 100644
--- a/main.c
+++ b/main.c
@@ -770,8 +770,29 @@ error:
  * \return don't return on sucess, -1 on error
  * \see main_loop
  */
+#include <assert.h>
 int main(int argc, char** argv)
 {
+       str s;
+       struct db_id* id;
+
+       /*init pkg mallocs (before parsing cfg but after parsing cmd line !)*/
+       if (init_pkg_mallocs()==-1)
+               goto error00;
+
+       // create a str object with a length of 23, but a longer string to
+       // simulate unallocated memory
+       //               11111111112222
+       //      12345678901234567890123
+       s.s  = "sqlite://tmp/0123456789ABCDEF";
+       s.len = 13          +10;
+
+       id = new_db_id(&s);
+
+       assert( strcmp(id->url, "sqlite://tmp/0123456789") == 0 );
+       exit(0);
+
+
```

---
Reply to this email directly or view it on GitHub:
https://github.com/OpenSIPS/opensips/issues/471#issuecomment-94652782
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.opensips.org/pipermail/devel/attachments/20150420/76440d1c/attachment.htm>