Hello,

any follow up on this one? It would be good to sort it out before next minor release.

Cheers, Daniel

On 04/09/15 12:21, Daniel-Constantin Mierla wrote:

One more thing:

some code related to a report on bug tracker was removed:

• memset(keywords, 0, (sizeof(char*) * 10));
• memset(values, 0, (sizeof(char*) * 10));
• memset(to, 0, (sizeof(char) * 16));

Was this intentional and related to your case? It is not related to columns from pg result.

Cheers, Daniel

On 04/09/15 12:07, Daniel-Constantin Mierla wrote:

...

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/74c84c7cd52347fcd1c90e75dca239b5...

Author: Carsten Bock carsten@ng-voice.com Committer: Carsten Bock carsten@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/74c84c7cd52347fcd1c90e75dca239b5... Patch: https://github.com/kamailio/kamailio/commit/74c84c7cd52347fcd1c90e75dca239b5...

---

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@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev