On Tue 26 Feb 2002 (21:58 +0100), Jim Segrave wrote:
>
> 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 **
I now have a patch against
/* $Id: php_mysql.c,v 1.97.2.3 2001/12/09 13:53:56 zeev Exp $ */
which appears to do what's needed. Initial testing indicates that it
blocks the exploit, and only allows load data local infile 'path'
queries where the path is valid for the php script owner (and I know a
bit more about the regex library). The same caveats still apply for a
threaded environment - the compile of the regex takes place on the
first query processed, so it would need mutex'ing if threaded.
Unified diff of a workding version follows:
========================================
--- php_mysql.c.save Wed Feb 27 13:22:35 2002
+++ php_mysql.c Wed Feb 27 12:16:00 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,122 @@
#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;
+ regmatch_t pmatch[2]; /* 0 = string, 1 = path */
+ 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, 2, 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 +1121,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
