Bug #73497 [Com]: memcache session handler with two backend servers Fatal Error (out of memory)

Edit report at https://bugs.php.net/bug.php?id=73497&edit=1 ID: 73497 Comment by: marcos dot gonzalez at bol dot com dot br Reported by: php at bof dot de Summary: memcache session handler with two backend servers Fatal Error (out of memory) Status: Open Type: Bug Package: Session related Operating System: openSUSE 11.4 + 13.1, x86_64 PHP Version: 5.6.28 Block user comment: N Private report: N New Comment: This bug also happens with redis as a backend for session handling. As long as you put multiple servers on session.save_path, separated by commas. Previous Comments: ------------------------------------------------------------------------ [2016-11-22 08:57:42] christian dot lechner at brain dot at This bug can also be reproduced with php 7.0.13 and memcache 3.0.9-dev ------------------------------------------------------------------------ [2016-11-16 09:55:38] php at bof dot de Re-reported against package "memcache (PECL)" in https://bugs.php.net/bug.php?id=73539 ------------------------------------------------------------------------ [2016-11-15 15:10:11] php at bof dot de The following patch fixes the issue for the memcache extension, by making a temporary copy of the single server parts of save_path before calling php_url_parse_ex: --- ../release-5.6.28//memcache/memcache_session.c 2016-11-10 16:27:45.963096097 +0100 +++ ./memcache/memcache_session.c 2016-11-15 16:07:09.186618020 +0100 @@ -90,7 +90,10 @@ efree(path); } else { - url = php_url_parse_ex(save_path+i, j-i); + int len = j-i; + char *path = estrndup(save_path+i, len); + url = php_url_parse_ex(path, strlen(path)); + efree(path); } if (!url) { ------------------------------------------------------------------------ [2016-11-15 14:48:44] php at bof dot de adding to my previous "analysis".... I was a bit confused because I found that I couldn't use the PHP level parse_url() function with the same argument as the memcache handler stuff, to get the same (bad result). Now, looking closely, the difference is that the memcache extension calls php_url_parse_ex with the two-server string BUT length SET TO 24, i.e. only covering the first of the two servers in the string. But then php_url_parse_ex obviously does not really limit itself to examining the passed-in length, and happily scans the string further. ------------------------------------------------------------------------ [2016-11-15 14:02:50] php at bof dot de Reproducing with my test script and an --enable-debug build, gives some hint: Fatal error: Allowed memory size of 134217728 bytes exhausted at /usr/src/phb/build/dbg-5.6.28/php-src/ext/standard/url.c:343 (tried to allocate 4294967291 bytes) in /tmp/badsess.php on line 37 url.c:343 is php_url_parse_ex() line ret->path = estrndup(s, (ue-s)); Trying to run under gdb with a breakpoint set there, gives the error in a different place: Fatal error: Allowed memory size of 134217728 bytes exhausted at /usr/src/phb/build/dbg-5.6.28/php-src/ext/standard/url.c:339 (tried to allocate 4294965800 bytes) in /tmp/badsess.php on line 37 That url.c:339 line is: ret->fragment = estrndup(p, (ue-p)); Also setting a breakpoint there. Result: Breakpoint 2, php_url_parse_ex ( str=0x7ffff7fcc830 "tcp://192.168.8.57:11211, tcp://192.168.8.58:11211", length=24) at /usr/src/phb/build/dbg-5.6.28/php-src/ext/standard/url.c:339 339 ret->fragment = estrndup(p, (ue-p)); The plot thickens... :) (gdb) print p $1 = 0x7ffff7fcce21 "\001" (gdb) print ue $2 = 0x7ffff7fcc848 ", tcp://192.168.8.58:11211" (gdb) print ue-p $3 = -1497 That "p" is "(p = memchr(s, '#', (ue - s)))" from line 329. (gdb) print s $4 = 0x7ffff7fcc84e "//192.168.8.58:11211" (gdb) print ue-s $5 = -6 So we have a memchr() call already with negative length (probably undefined behaviour). Here is the change between 5.6.27 and 5.6.28: --- ../release-5.6.27/php-src/ext/standard/url.c 2016-10-20 17:59:31.906609491 +0200 +++ ./php-src/ext/standard/url.c 2016-11-14 19:01:25.283278279 +0100 @@ -217,28 +217,7 @@ goto nohost; } - e = ue; - - if (!(p = memchr(s, '/', (ue - s)))) { - char *query, *fragment; - - query = memchr(s, '?', (ue - s)); - fragment = memchr(s, '#', (ue - s)); - - if (query && fragment) { - if (query > fragment) { - e = fragment; - } else { - e = query; - } - } else if (query) { - e = query; - } else if (fragment) { - e = fragment; - } - } else { - e = p; - } + e = s + strcspn(s, "/?#"); /* check for login and password */ if ((p = zend_memrchr(s, '@', (e-s)))) { Given the input string "tcp://192.168.8.57:11211, tcp://192.168.8.58:11211" and "s" already past the initial "tcp://", the old code would have found the '/', then wouldn't have found '?' or '#' - AND THUS WOULD HAVE LEFT "e" alone. After that change, "e" will be at the first '/' of the SECOND "argument", so that the code further effectively tries to parse "192.168.8.57:11211, tcp:" and then goes astray.... ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=73497 -- Edit this bug report at https://bugs.php.net/bug.php?id=73497&edit=1