Just a quick reminder for all of the developers, RC3 (the final RC)
will be tagged tomorrow afternoon (EST) time, please try to get
whatever patches (bug fixes only, please) you still want into it. If
there are reasons to delay, critical patches missing, please let me
know.
Thanks,
Ilia Alshanetsky
5.2 Release Master
Ilia Alshanetsky wrote:
Just a quick reminder for all of the developers, RC3 (the final RC) will
be tagged tomorrow afternoon (EST) time, please try to get whatever
patches (bug fixes only, please) you still want into it. If there are
reasons to delay, critical patches missing, please let me know.
http://bugs.php.net/bug.php?id=44938&edit=1
Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..
--Jani
The problem here is that the domain parameter will eventually make its
way into the file path and result in an overflow of the MAX_PATHLEN
there (I suspect). The problem however (IMHO) lies in gettext that
does not check the file path length before using it. For us to do the
check will require an arbitrary limit, since the domain comprises only
of a portion of the path and just putting a limit on its length of
MAX_PATHLEN would not be enough.
Ilia Alshanetsky wrote:
Just a quick reminder for all of the developers, RC3 (the final RC)
will be tagged tomorrow afternoon (EST) time, please try to get
whatever patches (bug fixes only, please) you still want into it.
If there are reasons to delay, critical patches missing, please let
me know.http://bugs.php.net/bug.php?id=44938&edit=1
Marked critical but propably isn't. Also depatable whether it's a
PHP bug at all..--Jani
Ilia Alshanetsky
Jani Taskinen wrote:
http://bugs.php.net/bug.php?id=44938&edit=1
Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..
Probably the library exhausting memory using alloca I'd say.
I whipped together a little patch against HEAD which restricts the
length of a text domain string to 10000 bytes to avoid problems in the
underlying library.
Note: I haven't been able to compile HEAD right now so I couldn't test
the patch really. But it compiles and should be rather trivial to
review/test.
- Chris
Not a bad idea, although I'd put a limit of the domain length o 4kb. I
can't see someone needing a domain longer then that, heck even 1kb
should be more then enough.
Jani Taskinen wrote:
http://bugs.php.net/bug.php?id=44938&edit=1
Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..Probably the library exhausting memory using alloca I'd say.
I whipped together a little patch against HEAD which restricts the
length of a text domain string to 10000 bytes to avoid problems in the
underlying library.Note: I haven't been able to compile HEAD right now so I couldn't test
the patch really. But it compiles and should be rather trivial to
review/test.
- Chris
Index: ext/gettext/gettext.c
===================================================================
RCS file: /repository/php-src/ext/gettext/gettext.c,v
retrieving revision 1.58
diff -u -r1.58 gettext.c
--- ext/gettext/gettext.c 24 Oct 2008 14:34:13 -0000 1.58
+++ ext/gettext/gettext.c 29 Oct 2008 13:47:15 -0000
@@ -30,6 +30,8 @@
#include "ext/standard/info.h"
#include "php_gettext.h"+#define MAX_DOMAIN_LENGTH 10000 /* Maximum length of textdomain
name length */
/* {{{ arginfo */
ZEND_BEGIN_ARG_INFO(arginfo_textdomain, 0)
ZEND_ARG_INFO(0, domain)
@@ -162,6 +164,10 @@
return;
}
- if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;- }
if (!domain_len || (domain_len == 1 && *domain_str == '0')) {
domain_str = NULL;
}
@@ -193,6 +199,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv))) {
return;
}- if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;- }
RETURN_STRING(dgettext(domain_str, msgid_str), ZSTR_DUPLICATE);
}
/* }}} */
@@ -208,6 +218,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&l", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv), &category)) {
return;
}- if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;}
RETURN_STRING(dcgettext(domain_str, msgid_str, category),
ZSTR_DUPLICATE);
}
/* }}} */
@@ -223,6 +237,10 @@
return;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong");
RETURN_FALSE;}
if (!domain_len) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "the first parameter
must not be empty");
RETURN_FALSE;
@@ -273,6 +291,10 @@
RETURN_FALSE;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;}
if ((msgstr = dngettext(domain_str, msgid_str1, msgid_str2,
count))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -295,6 +317,10 @@
RETURN_FALSE;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;}
if ((msgstr = dcngettext(domain_str, msgid_str1, msgid_str2, count,
category))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -316,6 +342,10 @@
return;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong");
RETURN_FALSE;- }
if (!codeset_len) {
codeset_str = NULL;
}--
Ilia Alshanetsky
Christian,
Your patch with slight revisions just went in.
Jani Taskinen wrote:
http://bugs.php.net/bug.php?id=44938&edit=1
Marked critical but propably isn't. Also depatable whether it's a PHP
bug at all..Probably the library exhausting memory using alloca I'd say.
I whipped together a little patch against HEAD which restricts the
length of a text domain string to 10000 bytes to avoid problems in the
underlying library.Note: I haven't been able to compile HEAD right now so I couldn't test
the patch really. But it compiles and should be rather trivial to
review/test.
- Chris
Index: ext/gettext/gettext.c
===================================================================
RCS file: /repository/php-src/ext/gettext/gettext.c,v
retrieving revision 1.58
diff -u -r1.58 gettext.c
--- ext/gettext/gettext.c 24 Oct 2008 14:34:13 -0000 1.58
+++ ext/gettext/gettext.c 29 Oct 2008 13:47:15 -0000
@@ -30,6 +30,8 @@
#include "ext/standard/info.h"
#include "php_gettext.h"+#define MAX_DOMAIN_LENGTH 10000 /* Maximum length of textdomain
name length */
/* {{{ arginfo */
ZEND_BEGIN_ARG_INFO(arginfo_textdomain, 0)
ZEND_ARG_INFO(0, domain)
@@ -162,6 +164,10 @@
return;
}
- if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;- }
if (!domain_len || (domain_len == 1 && *domain_str == '0')) {
domain_str = NULL;
}
@@ -193,6 +199,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv))) {
return;
}- if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;- }
RETURN_STRING(dgettext(domain_str, msgid_str), ZSTR_DUPLICATE);
}
/* }}} */
@@ -208,6 +218,10 @@
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,
"s&s&l", &domain_str, &domain_len,
ZEND_U_CONVERTER(UG(filesystem_encoding_conv)), &msgid_str,
&msgid_len, UG(ascii_conv), &category)) {
return;
}- if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;}
RETURN_STRING(dcgettext(domain_str, msgid_str, category),
ZSTR_DUPLICATE);
}
/* }}} */
@@ -223,6 +237,10 @@
return;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong");
RETURN_FALSE;}
if (!domain_len) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "the first parameter
must not be empty");
RETURN_FALSE;
@@ -273,6 +291,10 @@
RETURN_FALSE;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;}
if ((msgstr = dngettext(domain_str, msgid_str1, msgid_str2,
count))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -295,6 +317,10 @@
RETURN_FALSE;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong, ignoring");
domain_str = NULL;}
if ((msgstr = dcngettext(domain_str, msgid_str1, msgid_str2, count,
category))) {
RETURN_STRING(msgstr, ZSTR_DUPLICATE);
} else {
@@ -316,6 +342,10 @@
return;
}if (domain_len > MAX_DOMAIN_LENGTH) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "domain passed toolong");
RETURN_FALSE;- }
if (!codeset_len) {
codeset_str = NULL;
}--
Ilia Alshanetsky
Hi Illia,
Am Mittwoch, den 29.10.2008, 17:02 -0400 schrieb Ilia Alshanetsky:
[...]
Your patch with slight revisions just went in.
The sad thing is, that the patch just fixes half of the issue. If your
try the examples in the bugreport, most of the still work. We have also
check the msgids for overflows. I'm currently working on a patch
including tests.
cu, Lars
Just a quick reminder for all of the developers, RC3 (the final RC)
will be tagged tomorrow afternoon (EST) time, please try to get
whatever patches (bug fixes only, please) you still want into it. If
there are reasons to delay, critical patches missing, please let me
know.
Can we do on Tuesday next week instead.?
Many of us are in the IPC conference these days and won't be back in
normal activities until Monday. Doing so will also let me be sure that
everything works on windows before you tag this release.
Cheers,
Pierre