Hi list,
according to bug #258 we don't use the string character escaping functions from
mysql client library to ensure the safety of the SQL statements.
Attached is a patch for gw/dlr_mysql.c:dlr_mysql_add() which uses a static
mysql_escaipe() that does this. Please review and vote for commitment. This
should be also extended to the other SQL statements, right?
Actually this is a bit "glitchy", since we have to consume the DBPoolConn*
_before_ passing the strings to the mysql_real_escape_string() routine. It
expects a mysql connection in order to ensure that it gets the right character
set encoding.
Anyone having a better "way" in doing this? I don't feel it's the "smoothes"
code for it, but it safes at least a sub-sequential consumption of DBPoolConn's
while doing the string escaping.
Any improvments welcome.
Stipe
mailto:stolj_{at}_wapme-group.de
-------------------------------------------------------------------
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
-------------------------------------------------------------------
Index: gw/dlr_mysql.c
===================================================================
RCS file: /home/cvs/gateway/gw/dlr_mysql.c,v
retrieving revision 1.10
diff -u -r1.10 dlr_mysql.c
--- gw/dlr_mysql.c 21 Sep 2005 02:01:22 -0000 1.10
+++ gw/dlr_mysql.c 26 Sep 2005 17:04:14 -0000
@@ -58,12 +58,12 @@
* dlr_mysql.c
*
* Implementation of handling delivery reports (DLRs)
- * for MySql database
+ * for MySQL database
*
* Andreas Fink <[EMAIL PROTECTED]>, 18.08.2001
* Stipe Tolj <[EMAIL PROTECTED]>, 22.03.2002
* Alexander Malysh <[EMAIL PROTECTED]> 2003
-*/
+ */
#include "gwlib/gwlib.h"
#include "gwlib/dbpool.h"
@@ -84,19 +84,20 @@
static struct dlr_db_fields *fields = NULL;
-static void mysql_update(const Octstr *sql)
+static void mysql_update(DBPoolConn *pc, const Octstr *sql)
{
int state;
- DBPoolConn *pc;
#if defined(DLR_TRACE)
- debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
+ debug("dlr.mysql", 0, "SQL: %s", octstr_get_cstr(sql));
#endif
- pc = dbpool_conn_consume(pool);
+ /* if none connection has been passed get one */
if (pc == NULL) {
- error(0, "MYSQL: Database pool got no connection! DB update failed!");
- return;
+ if ((pc = dbpool_conn_consume(pool)) == NULL) {
+ error(0, "MYSQL: Database pool got no connection! DB update
failed!");
+ return;
+ }
}
state = mysql_query(pc->conn, octstr_get_cstr(sql));
@@ -106,19 +107,20 @@
dbpool_conn_produce(pc);
}
-static MYSQL_RES* mysql_select(const Octstr *sql)
+
+static MYSQL_RES *mysql_select(const Octstr *sql)
{
int state;
MYSQL_RES *result = NULL;
DBPoolConn *pc;
#if defined(DLR_TRACE)
- debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
+ debug("dlr.mysql", 0, "SQL: %s", octstr_get_cstr(sql));
#endif
pc = dbpool_conn_consume(pool);
if (pc == NULL) {
- error(0, "MYSQL: Database pool got no connection! DB update failed!");
+ error(0, "MYSQL: Database pool got no connection! DB select failed!");
return NULL;
}
@@ -134,16 +136,58 @@
return result;
}
+
+static Octstr *mysql_escape(DBPoolConn *pc, const Octstr *os)
+{
+ Octstr *sql_safe_os;
+ char *sql_safe;
+
+ /* The worst case is that every char is escaped to two bytes,
+ * plus the trailing \0 character. */
+ sql_safe = gw_malloc(octstr_len(os) * 2 + 1);
+ gw_assert(sql_safe != NULL);
+
+ mysql_real_escape_string(pc->conn, sql_safe, octstr_get_cstr(os),
octstr_len(os));
+
+ sql_safe_os = sql_safe ? octstr_create(sql_safe) : NULL;
+ gw_free(sql_safe);
+
+ return sql_safe_os;
+}
+
+
static void dlr_mysql_shutdown()
{
dbpool_destroy(pool);
dlr_db_fields_destroy(fields);
}
+
static void dlr_mysql_add(struct dlr_entry *entry)
{
Octstr *sql;
+ Octstr *smsc, *timestamp, *source, *destination, *service, *url, *boxc_id;
+ DBPoolConn *pc;
+
+ /* We need to aquire a mysql connction prior due to the call to
+ * mysql_real_escape_string(), which requires the connection in order
+ * to determine used character set. */
+ if ((pc = dbpool_conn_consume(pool)) == NULL) {
+ error(0, "MYSQL: Database pool got no connection! DB update failed!");
+ goto error;
+ }
+
+ /* For the sake of safety: make sure we escape all strings
+ * that come arround from outside */
+ smsc = mysql_escape(pc, entry->smsc);
+ timestamp = mysql_escape(pc, entry->timestamp);
+ source = mysql_escape(pc, entry->source);
+ destination = mysql_escape(pc, entry->destination);
+ service = mysql_escape(pc, entry->service);
+ url = mysql_escape(pc, entry->url);
+ boxc_id = mysql_escape(pc, entry->boxc_id);
+ /* prepare insert statement */
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');",
octstr_get_cstr(fields->table),
octstr_get_cstr(fields->field_smsc),
@@ -152,35 +196,45 @@
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);
+ octstr_get_cstr(smsc), octstr_get_cstr(timestamp),
octstr_get_cstr(source),
+ octstr_get_cstr(destination),
octstr_get_cstr(service), octstr_get_cstr(url),
+ entry->mask, octstr_get_cstr(boxc_id), 0);
+
+ octstr_destroy(smsc);
+ octstr_destroy(timestamp);
+ octstr_destroy(source);
+ octstr_destroy(destination);
+ octstr_destroy(service);
+ octstr_destroy(url);
+ octstr_destroy(boxc_id);
+ mysql_update(pc, sql);
- mysql_update(sql);
-
+error:
octstr_destroy(sql);
dlr_entry_destroy(entry);
}
-static struct dlr_entry* dlr_mysql_get(const Octstr *smsc, const Octstr *ts,
const Octstr *dst)
+
+static struct dlr_entry *dlr_mysql_get(const Octstr *smsc, const Octstr *ts,
const Octstr *dst)
{
struct dlr_entry *res = NULL;
Octstr *sql;
- MYSQL_RES *result;
+ MYSQL_RES *result = NULL;
MYSQL_ROW row;
-
+
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));
-
+ octstr_get_cstr(smsc),
octstr_get_cstr(fields->field_ts),
+ octstr_get_cstr(ts));
result = mysql_select(sql);
octstr_destroy(sql);
+error:
if (result == NULL) {
return NULL;
}
@@ -214,6 +268,7 @@
return res;
}
+
static void dlr_mysql_remove(const Octstr *smsc, const Octstr *ts, const
Octstr *dst)
{
Octstr *sql;
@@ -224,11 +279,12 @@
octstr_get_cstr(smsc),
octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
- mysql_update(sql);
+ mysql_update(NULL, sql);
octstr_destroy(sql);
}
+
static void dlr_mysql_update(const Octstr *smsc, const Octstr *ts, const
Octstr *dst, int status)
{
Octstr *sql;
@@ -240,7 +296,7 @@
octstr_get_cstr(fields->field_smsc),
octstr_get_cstr(smsc),
octstr_get_cstr(fields->field_ts),
octstr_get_cstr(ts));
- mysql_update(sql);
+ mysql_update(NULL, sql);
octstr_destroy(sql);
}
@@ -278,16 +334,18 @@
return res;
}
+
static void dlr_mysql_flush(void)
{
Octstr *sql;
sql = octstr_format("DELETE FROM %s;", octstr_get_cstr(fields->table));
- mysql_update(sql);
+ mysql_update(NULL, sql);
octstr_destroy(sql);
}
+
static struct dlr_storage handles = {
.type = "mysql",
.dlr_add = dlr_mysql_add,
@@ -299,6 +357,7 @@
.dlr_flush = dlr_mysql_flush
};
+
struct dlr_storage *dlr_init_mysql(Cfg *cfg)
{
CfgGroup *grp;
