tags 382607 patch
clone 382607 -1
reassign -1 pike7.6
found -1 7.6.24
notfound -1 7.6.87
thanks
(some bts-fu to reflect the bug exists in both versions of pike)
executive summary for security team: not escaping query strings
can possibly result in SQL injection for apps that use pike+postgresql.
i've developed a patch which cleanly applies to both the 7.2 and 7.6
branches that exist in sarge. however, looking more closely at
what the patch actually *does*, it seems that it does nothing
but provide a new function which can do the escaping of query strings.
(patch attached)
therefore, i'm not sure that this is even really a valid security issue
worth an update, at least by itself. unless this function is somehow
automatically invoked when code uses the postgres module, it has no
effect on any application that uses pike/postgres, and thus has no
improvement on security apart from offering pike developers in sarge
access to the function in question.
so, i propose that either (a) in addition to supplying the fix we audit
all pike+postgres apps using postgresql, or (b) consider dropping the
issue entirely.
okay, i just did (a), and it doesn't look like there are any
pike+postgres apps in the archive for sarge at all. so given that,
what say you security team?
sean
ps - fwiw pike seems to FTBFS for me in sarge right now, so i don't have
a working NMU diff.gz
--
--- old/src/modules/Postgres/postgres.c 2004-02-03 10:13:58.0 +
+++ new/src/modules/Postgres/postgres.c 2006-08-19 09:37:09.0 +
@@ -728,6 +728,33 @@
Pike_error (Bad connection.\n);
}
+/*! @decl string quote(string s)
+ *!
+ *! Escape a string to prevent SQL injection, using the current connection's
+ *! character encoding settings.
+ */
+static void f_quote(INT32 args)
+{
+ int err;
+ int len;
+ struct pike_string *ret;
+ struct pike_string *s;
+ char *err_msg;
+
+ get_all_args(Postgres-quote, args, %S, s);
+
+ ret = begin_shared_string(s-len * 2 + 1);
+ len = PQescapeStringConn(THIS-dblink, ret-str, s-str, s-len, err);
+ if (err != 0) {
+ err_msg = PQerrorMessage(THIS-dblink);
+ set_error(err_msg);
+ Pike_error(err_msg);
+ }
+ pop_n_elems(args);
+ push_string(end_and_resize_shared_string(ret, len));
+}
+
+
/*! @endclass
*!
*! @endmodule
@@ -762,6 +795,9 @@
ADD_FUNCTION(host_info,f_host_info,tFunc(tVoid,tStr),
OPT_EXTERNAL_DEPEND|OPT_RETURN);
+ /* function(string:string) */
+ ADD_FUNCTION(quote, f_quote, tFunc(tStr,tStr), 0);
+
/* postgres-specific functions */
/* function(void:void) */
ADD_FUNCTION(reset,f_reset,tFunc(tVoid,tVoid),OPT_EXTERNAL_DEPEND|OPT_SIDE_EFFECT);
signature.asc
Description: Digital signature