Hi list,
we have a possible bug situation in the gw/dlr_mysql.c module, especially for
the dlr_mysql_add() function:
We perform an INSERT into the table space with the values provided in dlr_entry
struct. The field entry->source is the source address and if we used an
alphanumeric value here containing any SQL "administrative chars" (i.e. ') we
run into an mysql error. We need to ensure that all values passed from the
"outside" (via smsbox HTTP interface) to the SQL creation format is passed via
mysql's mysql_escape_string() function, ensuring such chars are escaped.
We had such a patch posted by:
From: Peter Christensen
Subject: Re: dlr_mysql_add and internal charset
Date: Tue, 10 Jan 2006 07:44:02 -0800
URL: http://www.mail-archive.com/[email protected]/msg05381.html
but it was actually never applied. This is a re-write of Peter's patch, making a
dbpool_mysql_escape_string() wrapper function available in the
gwlib/dbpool_mysql.c and using it in gw/dlr_mysql.c.
Please review and vote for committing to CVS.
Stipe
-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany
tolj.org system architecture Kannel Software Foundation (KSF)
http://www.tolj.org/ http://www.kannel.org/
mailto:st_{at}_tolj.org mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------
### Eclipse Workspace Patch 1.0
#P gateway-cvs-head
Index: gw/dlr_mysql.c
===================================================================
RCS file: /home/cvs/gateway/gw/dlr_mysql.c,v
retrieving revision 1.14
diff -u -r1.14 dlr_mysql.c
--- gw/dlr_mysql.c 9 Jan 2008 20:06:58 -0000 1.14
+++ gw/dlr_mysql.c 9 Aug 2008 20:29:01 -0000
@@ -83,6 +83,11 @@
*/
static struct dlr_db_fields *fields = NULL;
+/*
+ * Function wrapper for mysql_escape_string().
+ */
+char *dbpool_mysql_escape_string(const char *string);
+
static void mysql_update(const Octstr *sql)
{
@@ -143,6 +148,26 @@
static void dlr_mysql_add(struct dlr_entry *entry)
{
Octstr *sql;
+ char *safe_ts, *safe_src, *safe_url, *safe_smsc;
+ char *safe_dst, *safe_srv, *safe_boxcid;
+
+ safe_ts = safe_src = safe_url = safe_smsc = safe_dst = safe_srv =
safe_boxcid = NULL;
+
+ /* ensure we handle only strings passed via mysql_escape_string() */
+ if ((safe_ts =
dbpool_mysql_escape_string(octstr_get_cstr(entry->timestamp))) == NULL)
+ goto done;
+ if ((safe_src =
dbpool_mysql_escape_string(octstr_get_cstr(entry->source))) == NULL)
+ goto done;
+ if ((safe_url = dbpool_mysql_escape_string(octstr_get_cstr(entry->url)))
== NULL)
+ goto done;
+ if ((safe_smsc = dbpool_mysql_escape_string(octstr_get_cstr(entry->smsc)))
== NULL)
+ goto done;
+ if ((safe_dst =
dbpool_mysql_escape_string(octstr_get_cstr(entry->destination))) == NULL)
+ goto done;
+ if ((safe_srv =
dbpool_mysql_escape_string(octstr_get_cstr(entry->service))) == NULL)
+ goto done;
+ if ((safe_boxcid =
dbpool_mysql_escape_string(octstr_get_cstr(entry->boxc_id))) == NULL)
+ goto done;
sql = octstr_format("INSERT INTO %s (%s, %s, %s, %s, %s, %s, %s, %s, %s)
VALUES "
"('%s', '%s', '%s', '%s', '%s', '%s', '%d', '%s',
'%d');",
@@ -152,14 +177,24 @@
octstr_get_cstr(fields->field_serv),
octstr_get_cstr(fields->field_url),
octstr_get_cstr(fields->field_mask),
octstr_get_cstr(fields->field_boxc),
octstr_get_cstr(fields->field_status),
- octstr_get_cstr(entry->smsc),
octstr_get_cstr(entry->timestamp), octstr_get_cstr(entry->source),
- octstr_get_cstr(entry->destination),
octstr_get_cstr(entry->service), octstr_get_cstr(entry->url),
- entry->mask, octstr_get_cstr(entry->boxc_id), 0);
+ safe_smsc, safe_ts, safe_src,
+ safe_dst, safe_srv, safe_url,
+ entry->mask, safe_boxcid, 0);
mysql_update(sql);
octstr_destroy(sql);
+
+done:
+ gw_free(safe_ts);
+ gw_free(safe_src);
+ gw_free(safe_url);
+ gw_free(safe_smsc);
+ gw_free(safe_dst);
+ gw_free(safe_srv);
+ gw_free(safe_boxcid);
+
dlr_entry_destroy(entry);
}
@@ -169,31 +204,41 @@
Octstr *sql;
MYSQL_RES *result;
MYSQL_ROW row;
+ char *safe_smsc, *safe_ts, *safe_dst;
+
+ safe_smsc = safe_ts = safe_dst = NULL;
+
+ /* ensure we handle only strings passed via mysql_escape_string() */
+ if ((safe_ts = dbpool_mysql_escape_string(octstr_get_cstr(ts))) == NULL)
+ goto done;
+ if ((safe_smsc = dbpool_mysql_escape_string(octstr_get_cstr(smsc))) ==
NULL)
+ goto done;
+ if ((safe_dst = dbpool_mysql_escape_string(octstr_get_cstr(dst))) == NULL)
+ goto done;
sql = octstr_format("SELECT %s, %s, %s, %s, %s, %s FROM %s WHERE %s='%s'
AND %s='%s';",
octstr_get_cstr(fields->field_mask),
octstr_get_cstr(fields->field_serv),
octstr_get_cstr(fields->field_url),
octstr_get_cstr(fields->field_src),
octstr_get_cstr(fields->field_dst),
octstr_get_cstr(fields->field_boxc),
octstr_get_cstr(fields->table),
octstr_get_cstr(fields->field_smsc),
- octstr_get_cstr(smsc),
octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
-
+ safe_smsc, octstr_get_cstr(fields->field_ts), safe_ts);
result = mysql_select(sql);
octstr_destroy(sql);
if (result == NULL) {
- return NULL;
+ goto done;
}
if (mysql_num_rows(result) < 1) {
debug("dlr.mysql", 0, "no rows found");
mysql_free_result(result);
- return NULL;
+ goto done;
}
row = mysql_fetch_row(result);
if (!row) {
debug("dlr.mysql", 0, "rows found but could not load them");
mysql_free_result(result);
- return NULL;
+ goto done;
}
debug("dlr.mysql", 0, "Found entry, row[0]=%s, row[1]=%s, row[2]=%s,
row[3]=%s, row[4]=%s row[5]=%s",
@@ -211,38 +256,74 @@
mysql_free_result(result);
+done:
+ gw_free(safe_ts);
+ gw_free(safe_smsc);
+ gw_free(safe_dst);
+
return res;
}
static void dlr_mysql_remove(const Octstr *smsc, const Octstr *ts, const
Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql = NULL;
+ char *safe_smsc, *safe_ts, *safe_dst;
+
+ safe_smsc = safe_ts = safe_dst = NULL;
+
+ /* ensure we handle only strings passed via mysql_escape_string() */
+ if ((safe_ts = dbpool_mysql_escape_string(octstr_get_cstr(ts))) == NULL)
+ goto done;
+ if ((safe_smsc = dbpool_mysql_escape_string(octstr_get_cstr(smsc))) ==
NULL)
+ goto done;
+ if ((safe_dst = dbpool_mysql_escape_string(octstr_get_cstr(dst))) == NULL)
+ goto done;
debug("dlr.mysql", 0, "removing DLR from database");
sql = octstr_format("DELETE FROM %s WHERE %s='%s' AND %s='%s' LIMIT 1;",
octstr_get_cstr(fields->table),
octstr_get_cstr(fields->field_smsc),
- octstr_get_cstr(smsc),
octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
-
+ safe_smsc, octstr_get_cstr(fields->field_ts), safe_ts);
mysql_update(sql);
octstr_destroy(sql);
+
+done:
+ gw_free(safe_ts);
+ gw_free(safe_smsc);
+ gw_free(safe_dst);
}
static void dlr_mysql_update(const Octstr *smsc, const Octstr *ts, const
Octstr *dst, int status)
{
Octstr *sql;
+ char *safe_smsc, *safe_ts, *safe_dst;
+
+ safe_smsc = safe_ts = safe_dst = NULL;
+
+ /* ensure we handle only strings passed via mysql_escape_string() */
+ if ((safe_ts = dbpool_mysql_escape_string(octstr_get_cstr(ts))) == NULL)
+ goto done;
+ if ((safe_smsc = dbpool_mysql_escape_string(octstr_get_cstr(smsc))) ==
NULL)
+ goto done;
+ if ((safe_dst = dbpool_mysql_escape_string(octstr_get_cstr(dst))) == NULL)
+ goto done;
debug("dlr.mysql", 0, "updating DLR status in database");
sql = octstr_format("UPDATE %s SET %s=%d WHERE %s='%s' AND %s='%s' LIMIT
1;",
octstr_get_cstr(fields->table),
octstr_get_cstr(fields->field_status), status,
- octstr_get_cstr(fields->field_smsc),
octstr_get_cstr(smsc),
- octstr_get_cstr(fields->field_ts),
octstr_get_cstr(ts));
+ octstr_get_cstr(fields->field_smsc), safe_smsc,
+ octstr_get_cstr(fields->field_ts), safe_ts);
mysql_update(sql);
octstr_destroy(sql);
+
+done:
+ gw_free(safe_ts);
+ gw_free(safe_smsc);
+ gw_free(safe_dst);
}
Index: gwlib/dbpool_mysql.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/dbpool_mysql.c,v
retrieving revision 1.8
diff -u -r1.8 dbpool_mysql.c
--- gwlib/dbpool_mysql.c 9 Jan 2008 20:06:55 -0000 1.8
+++ gwlib/dbpool_mysql.c 9 Aug 2008 20:29:02 -0000
@@ -67,6 +67,22 @@
#include <mysql.h>
+char *dbpool_mysql_escape_string(const char *string)
+{
+ unsigned long int size, length;
+ char *buffer;
+ length = strlen(string);
+ size = (strlen(string) << 1) + 1;
+ if ((buffer = (char*)gw_malloc(size)) == NULL) {
+ error(0, "MYSQL: Error allocating buffer for string");
+ return NULL;
+ } else {
+ mysql_escape_string(buffer, string, length);
+ return buffer;
+ }
+}
+
+
static void *mysql_open_conn(const DBConf *db_conf)
{
MYSQL *mysql = NULL;
