Edit report at https://bugs.php.net/bug.php?id=60339&edit=1
ID: 60339
Comment by: yohgaki at ohgaki dot net
Reported by: yohgaki at ohgaki dot net
Summary: valgrind reports LEAK --with-mm
Status: Open
Type: Bug
Package: Session related
Operating System: Linux
PHP Version: 5.4.0RC1
Block user comment: N
Private report: N
New Comment:
I've tested as follows.
(from my bash history)
1004 tar zxvf ../Download/php-5.4.0RC1.tar.bz2
1005 cd php-5.4.0RC1/
1006 ./configure --with-mm && make -j 8
1007 TEST_PHP_EXECUTABLE="./sapi/cli/php" ./run-tests.php -m ext/session/
Felipe, if you could commit the patch, I appreciated it.
If you take a look at PHP_MINIT_FUNCTION(ps_mm) in ext/session/mod_mm.c, you'll
see it will underflow by 1 byte when strlen(PS(save_path)) equals 0.
Previous Comments:
------------------------------------------------------------------------
[2011-11-19 18:20:44] yohgaki at ohgaki dot net
Since my tree is full of changes for strict session patch.
Could anyone commit patch for this bug?
Patch should be applied to php-src, php-src-5.4 and php-src-5.3.
I'm not sure if this bug is exploitable with current memory manager. Since
DEFAULT_SLASH would be ascii 47 or 97, it would be difficult.
------------------------------------------------------------------------
[2011-11-19 18:10:06] fel...@php.net
I can't reproduce it, are you using any .INI?
=====================================================================
PHP : sapi/cli/php
PHP_SAPI : cli
PHP_VERSION : 5.4.0RC2-dev
ZEND_VERSION: 2.4.0
PHP_OS : Linux - Linux sig11 2.6.32-5-amd64 #1 SMP Mon Oct 3 03:59:20 UTC
2011 x86_64
INI actual : /home/felipe/dev/php5_4
More .INIs :
CWD : /home/felipe/dev/php5_4
Extra dirs :
VALGRIND : valgrind-3.6.0.SVN-Debian
=====================================================================
------------------------------------------------------------------------
[2011-11-19 17:59:23] yohgaki at ohgaki dot net
I figured out the cause. This is simple underflow. The correct patch is this.
--- mod_mm.c (ãªãã¸ã§ã³ 319529)
+++ mod_mm.c (ä½æ¥ã³ãã¼)
@@ -278,7 +278,7 @@
ps_mm_path = emalloc(save_path_len + 1 + (sizeof(PS_MM_FILE) - 1) +
mod_name_len + euid_len + 1);
memcpy(ps_mm_path, PS(save_path), save_path_len);
- if (PS(save_path)[save_path_len - 1] != DEFAULT_SLASH) {
+ if (save_path_len && PS(save_path)[save_path_len - 1] != DEFAULT_SLASH)
{
ps_mm_path[save_path_len] = DEFAULT_SLASH;
save_path_len++;
}
------------------------------------------------------------------------
[2011-11-19 17:53:07] yohgaki at ohgaki dot net
Adding extra byte to OnUpdateSaveDir() in session.c removes LEAKs
@@ -607,7 +613,7 @@
}
}
- OnUpdateString(entry, new_value, new_value_length, mh_arg1, mh_arg2,
mh_arg3, stage TSRMLS_CC);
+ OnUpdateString(entry, new_value, new_value_length+1, mh_arg1, mh_arg2,
mh_arg3, stage TSRMLS_CC);
return SUCCESS;
}
/* }}} */
but it does not seem to be a correct fix for this.
------------------------------------------------------------------------
[2011-11-19 17:30:32] yohgaki at ohgaki dot net
Description:
------------
Valgrind reports LEAKs --with-mm
(php-src HEAD is affected, too. I noticed this while I'm making strict session
patch.)
=====================================================================
LEAKED TEST SUMMARY
---------------------------------------------------------------------
session rfc1867 [ext/session/tests/rfc1867.phpt]
session rfc1867 [ext/session/tests/rfc1867_cleanup.phpt]
session rfc1867 disabled [ext/session/tests/rfc1867_disabled.phpt]
session rfc1867 disabled 2 [ext/session/tests/rfc1867_disabled_2.phpt]
session rfc1867 [ext/session/tests/rfc1867_inter.phpt]
session rfc1867 no name [ext/session/tests/rfc1867_no_name.phpt]
session rfc1867 sid cookie [ext/session/tests/rfc1867_sid_cookie.phpt]
session rfc1867 sid get [ext/session/tests/rfc1867_sid_get.phpt]
session rfc1867 sid get 2 [ext/session/tests/rfc1867_sid_get_2.phpt]
session rfc1867 sid cookie [ext/session/tests/rfc1867_sid_invalid.phpt]
session rfc1867 sid only cookie [ext/session/tests/rfc1867_sid_only_cookie.phpt]
session rfc1867 sid only cookie 2
[ext/session/tests/rfc1867_sid_only_cookie_2.phpt]
session rfc1867 sid post [ext/session/tests/rfc1867_sid_post.phpt]
Test session_module_name() function : variation
[ext/session/tests/session_module_name_variation3.phpt]
Test session_name() function : error functionality
[ext/session/tests/session_name_basic.phpt]
Test session_name() function : error functionality
[ext/session/tests/session_name_error.phpt]
Test session_name() function : variation
[ext/session/tests/session_name_variation1.phpt]
Test session_save_path() function : basic functionality
[ext/session/tests/session_save_path_basic.phpt]
Test session_save_path() function : error functionality
[ext/session/tests/session_save_path_error.phpt]
Test session_save_path() function : variation
[ext/session/tests/session_save_path_variation1.phpt]
Test session_save_path() function : variation
[ext/session/tests/session_save_path_variation4.phpt]
Test session_save_path() function : variation
[ext/session/tests/session_save_path_variation5.phpt]
Test session_set_save_handler() function : basic functionality
[ext/session/tests/session_set_save_handler_basic.phpt]
Test session_set_save_handler() function : using closures as callbacks
[ext/session/tests/session_set_save_handler_closures.phpt]
Test session_set_save_handler() function : error functionality
[ext/session/tests/session_set_save_handler_error3.phpt]
Test session_set_save_handler() function : variation
[ext/session/tests/session_set_save_handler_variation4.phpt]
=====================================================================
All LEAK reports are the same.
******* ext/session/tests/rfc1867.mem ************
==29422== Invalid read of size 1
==29422== at 0x57D8D3: zm_startup_ps_mm (mod_mm.c:281)
==29422== by 0x578A94: zm_startup_session (session.c:2178)
==29422== by 0x6784D4: zend_startup_module_ex (zend_API.c:1653)
==29422== by 0x681304: zend_hash_apply (zend_hash.c:716)
==29422== by 0x67BB15: zend_startup_modules (zend_API.c:1780)
==29422== by 0x61C27B: php_module_startup (main.c:2132)
==29422== by 0x718F02: php_cgi_startup (cgi_main.c:931)
==29422== by 0x7195D2: main (cgi_main.c:1873)
==29422== Address 0x4fa31af is 1 bytes before a block of size 1 alloc'd
==29422== at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==29422== by 0x6516AD: zend_strndup (zend_alloc.c:2617)
==29422== by 0x62361B: php_ini_parser_cb (php_ini.c:274)
==29422== by 0x64B84E: ini_parse (zend_ini_parser.c:1669)
==29422== by 0x64BAED: zend_parse_ini_string (zend_ini_parser.c:348)
==29422== by 0x622B44: php_init_config (php_ini.c:722)
==29422== by 0x61C1C9: php_module_startup (main.c:2078)
==29422== by 0x718F02: php_cgi_startup (cgi_main.c:931)
==29422== by 0x7195D2: main (cgi_main.c:1873)
==29422==
Test script:
---------------
N/A
Expected result:
----------------
No LEAKs
Actual result:
--------------
LEAKs are reported
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=60339&edit=1
