Hello,
following the discussion started by Chris Double, I am not sure this is
the right fix. The column names are now allocated in pkg, but don't get
freed, so looks like this commit introduced a memory leak. See:
http://lists.sip-router.org/pipermail/sr-dev/2015-September/030565.html
I think the commit has to be reverted and then remove
db_postgres_free_query() at the end of db_postgres_store_result()
Not using postgres module, but this seems the right from c coding point
of view comparing with db_mysql.
Cheers,
Daniel
On 28/08/15 12:11, Carsten Bock wrote:
Module: kamailio
Branch: master
Commit: 74c84c7cd52347fcd1c90e75dca239b5f758169b
URL:
https://github.com/kamailio/kamailio/commit/74c84c7cd52347fcd1c90e75dca239b…
Author: Carsten Bock <carsten(a)ng-voice.com>
Committer: Carsten Bock <carsten(a)ng-voice.com>
Date: 2015-08-28T12:11:25+02:00
db_postgres: Fix heap use after free error in db_postgres module
The result structure for a query holds a pointer returned by
PQfname. When sql_do_query executes the query it gets this
database result structure returned but the PQfname pointer
has already been free'd by a call to db_postgres_free_query
from within db_postgres_store_result.
sql_do_query then tries to copy the free'd string into another
result structure resulting in a heap use after free.
The fix here copies the PQfname result.
Fix by Chris Double
---
Modified: modules/db_postgres/km_pg_con.c
Modified: modules/db_postgres/km_res.c
---
Diff:
https://github.com/kamailio/kamailio/commit/74c84c7cd52347fcd1c90e75dca239b…
Patch:
https://github.com/kamailio/kamailio/commit/74c84c7cd52347fcd1c90e75dca239b…
---
diff --git a/modules/db_postgres/km_pg_con.c b/modules/db_postgres/km_pg_con.c
index ec98add..d053c55 100644
--- a/modules/db_postgres/km_pg_con.c
+++ b/modules/db_postgres/km_pg_con.c
@@ -71,10 +71,6 @@ struct pg_con* db_postgres_new_connection(struct db_id* id)
memset(ptr, 0, sizeof(struct pg_con));
ptr->ref = 1;
- memset(keywords, 0, (sizeof(char*) * 10));
- memset(values, 0, (sizeof(char*) * 10));
- memset(to, 0, (sizeof(char) * 16));
-
if (id->port) {
ports = int2str(id->port, 0);
keywords[i] = "port";
diff --git a/modules/db_postgres/km_res.c b/modules/db_postgres/km_res.c
index e9aa232..912206b 100644
--- a/modules/db_postgres/km_res.c
+++ b/modules/db_postgres/km_res.c
@@ -126,8 +126,14 @@ int db_postgres_get_columns(const db1_con_t* _h, db1_res_t* _r)
RES_NAMES(_r)[col]);
/* The pointer that is here returned is part of the result structure. */
- RES_NAMES(_r)[col]->s = PQfname(CON_RESULT(_h), col);
- RES_NAMES(_r)[col]->len = strlen(PQfname(CON_RESULT(_h), col));
+ RES_NAMES(_r)[col]->s = pkg_malloc(strlen(PQfname(CON_RESULT(_h), col))+1);
+ if (! RES_NAMES(_r)[col]->s) {
+ LM_ERR("no private memory left\n");
+ db_free_columns(_r);
+ return -4;
+ }
+ strcpy(RES_NAMES(_r)[col]->s, PQfname(CON_RESULT(_h), col));
+ RES_NAMES(_r)[col]->len = strlen(RES_NAMES(_r)[col]->s);
LM_DBG("RES_NAMES(%p)[%d]=[%.*s]\n", RES_NAMES(_r)[col], col,
RES_NAMES(_r)[col]->len, RES_NAMES(_r)[col]->s);
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla
http://twitter.com/#!/miconda -
http://www.linkedin.com/in/miconda
Book: SIP Routing With Kamailio -
http://www.asipto.com