I was looking at the posting from "James E. Flemer" <[EMAIL PROTECTED]>
and decided to try adding a bit more to it.
** This code is compiled, but not tested **
As noted in the comments, it would need work in a threaded
environment. However, if anyone would like to look at this and see if
it addresses most of the problem, I'd be grateful.
My idea was to use a regex match for the keywords of a LOAD DATA LOCAL
INFILE in the mysql query and, if found, extract the path of the file
and see if the php script generating the request would be able to read
that file. The optimisation of compiling the regex once on the first
query processed would require locking in a threaded environment.
diff is against the php_mysql.c in the 4.1.1 distribution:
/* $Id: php_mysql.c,v 1.97.2.3 2001/12/09 13:53:56 zeev Exp $ */
--- php_mysql.c.save Tue Feb 26 16:20:20 2002
+++ php_mysql.c Tue Feb 26 19:11:18 2002
@@ -55,6 +55,9 @@
#include "php_ini.h"
+#include <regex.h>
+#include <syslog.h>
+
#if HAVE_MYSQL_MYSQL_H
# include <mysql/mysql.h>
#else
@@ -211,9 +214,121 @@
#define CHECK_LINK(link) { if (link==-1) { php_error(E_WARNING, "MySQL: A link to
the server could not be established"); RETURN_FALSE; } }
+/* see if a file in a query includedes
+ * LOAD DATA [LOW_PRIORITY | CONCURRENT] [LOCAL] INFILE "path"
+ * if so, see if we should allow it to be run (is the file specified by path
+ * valid for php
+ */
+
+static int php_mysql_safety_check(const char *p, int len);
+
+static volatile int regex_is_ready = 0;
+
+/* regex for mysql queries we want to be careful with. Note that the
+ * file path we're looking for is any file name beginning with a
+ * single or double quote and assumed to end with either quote type. This
+ * will fail for file names containing one or the other quote types. File
+ * names like this won't be protected from the exploit, but the risk is
+ * small IMHO
+ */
+
+static char* query_regex =
+".*[[:<:]]LOAD[[:>:]].*[[:<:]]DATA[[:>:]].*[[:<:]]LOCAL[[:>:]].*[[:<:]]INFILE[[:>:]][^\\\'\\\"]*[\\\'\\\"]\\([^\\\'\\\"]*\\).*";
+
+static regex_t preg;
+
+/* compile this regex on the first SQL query seen */
+static int init_regex (void)
+{
+ int res;
+
+ /*
+ * XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
+ * FIXME
+ * In a threaded environment, this would need a mutex and all
+ * sorts of other frippery. Then the result could be made
+ * available to all other threads.
+ */
+
+ res = regcomp (&preg, query_regex, REG_EXTENDED | REG_ICASE);
+ if (res != 0) {
+ syslog (LOG_ERR, "init_regex: regcomp returned %d", res);
+ return 0;
+ }
+
+ regex_is_ready = 1;
+}
+
+/* look for mysql queries in the form LOAD DATA LOCAL INFILE 'path'
+ * if found, accept them (return 1) if a php script would be allowed
+ * to read the file
+ */
+static int php_mysql_safety_check(const char *query, int len)
+{
+ int res;
+ char *copy;
+ size_t nmatch;
+ regmatch_t pmatch[1]; /* I know how big this is */
+ char *path;
+
+ /* only compile the regex once */
+ if (!regex_is_ready) {
+ if (init_regex () == 0) {
+ /* didn't compile - don't accept query - that encourages
+ fixing things */
+ return 0;
+ }
+ }
+
+ /* dunno if I can trust the query to be null terminated. If I can,
+ why is there a length?
+ */
+ if ((copy = emalloc (len + 1)) == 0) {
+ syslog (LOG_ERR, "php_mysql_safety_check: Can't malloc query space");
+
+ strncpy (copy, query, len);
+ }
+
+ /* don't allow queries with embedded nulls - to catch a potential
+ * nasty hiding of paths
+ */
+ if ((len - strlen (copy)) > 1)
+ return 0;
+
+ res = regexec (&preg, query, nmatch, pmatch, 0);
+
+ /* it's not one of the dangerous queries - let it pass */
+ if (res == REG_NOMATCH) {
+ efree (copy);
+ return 1;
+ }
+
+ /* regexec didn't like the query, so we don't either */
+ if (res != 0) {
+ efree (copy);
+ return 0;
+ }
+
+ /* it matched, pmatch[1].rm_so is the offset of the start of the path
+ and pmatch[1].rm_eo is the offset of the end of the path
+ */
+ path = copy + pmatch[1].rm_so;
+ *(copy + pmatch[1].rm_eo) = 0;
+
+ /* path is the file path that the LOAD DATA... will use */
+ res = php_checkuid (path, "r", 0);
+
+ /* php_checkuid returns 0 if it's forbidden */
+ if (res == 0) {
+ syslog (LOG_ERR, "php_mysql_safety_check: suspect query for %s", path);
+ }
+
+ efree (copy);
+ return res;
+}
+
/* {{{ _free_mysql_result
* This wrapper is required since mysql_free_result() returns an integer, and
- * thus, cannot be used directly
+ * thus, cannot be used directly
*/
static void _free_mysql_result(zend_rsrc_list_entry *rsrc TSRMLS_DC)
{
@@ -1005,6 +1120,15 @@
} while(0);
convert_to_string_ex(query);
+
+ /* disallow LOAD DATA LOCAL INFILE if safe_mode on */
+ if (PG(safe_mode)) {
+ if (!php_mysql_safety_check(Z_STRVAL_PP(query), Z_STRLEN_PP(query))) {
+ php_error(E_WARNING, "MySQL: Statement failed safe mode
+checks");
+ RETURN_FALSE;
+ }
+ }
+
/* mysql_query is binary unsafe, use mysql_real_query */
#if MYSQL_VERSION_ID > 32199
if (mysql_real_query(&mysql->conn, Z_STRVAL_PP(query), Z_STRLEN_PP(query))!=0)
{
--
Jim Segrave [EMAIL PROTECTED]
--
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php
