php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #70542 dcngettext test crash due to invalid "category" parameter
Submitted: 2015-09-21 08:47 UTC Modified: 2015-11-01 04:22 UTC
From: rainer dot jung at kippdata dot de Assigned:
Status: No Feedback Package: Gettext related
PHP Version: 7.0.0RC3 OS: Solaris
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2015-09-21 08:47 UTC] rainer dot jung at kippdata dot de
Description:
------------
ext/gettext/tests/dcngettext.php crashes (segmentation fault) on Solaris due to the invalid catagory parameter with value "-1". Linux has special provisions in the source code for invalid category values, but on Solaris invalid values make the test crash.

The original crash stack is:

 fee94ecc get_hashid (fdc5f328, fdc5f328, fdc5f328, ffffffff, ffffffff, 1)
 fee92fd4 dcngettext (fdc5f328, fdc5f328, fdc5f328, ffffffff, ffffffff, ffbfec08) + 94
 fef41a28 zif_dcngettext (fdc13170, fdc130d0, 5f, 8dfb0, 44138, 2) + 9c
 fe7d0370 ZEND_DO_ICALL_SPEC_HANDLER (fdc13020, fdc80508, feda83d8, fdc13170, fdc13020, fdc80508) + 50
 fe7bccd0 execute_ex (fdc13020, 0, ffffffff, fffffff8, 0, feda83d8) + 24
 fe822838 zend_execute (fdc6c1e0, 0, 0, 0, fdc13020, feda83d8) + 1c0
 fe77c450 zend_execute_scripts (8, 0, 3, 1, feda83d8, ffbff500) + a8
 fe7154f0 php_execute_script (ffbff500, 6, 0, 1, 0, feda8828) + 27c
 00014110 do_cli   (1, fffe6820, 1cf10, 13dc00, 3b7d8, 14) + 1098
 0001cac8 main     (0, 0, 5d, 1d258, 3741c, 1cc50) + 3e8
 000128f4 _start   (0, 0, 0, 0, 0, 0) + 5c

and the crash can easily reproduced by simply calling dcgettext("test", "test", -1) on Solaris (or dcngettext("test", "test", "test", -1, -1) if you like).

You might want to either use category value 0 which seems to exist for all platforms, but might change test output, or LC_ALL which IMHO returns the same result on Linux and is always valid. Unfortunately LC_ALL is not available in the gettext ext as a constant, so it might make sense (not only to fix this bug) to add the usual constants to the gettext extension. Looking at the standards documentation of locale.h that would be:

LC_ALL
LC_COLLATE
LC_CTYPE
LC_MESSAGES
LC_MONETARY
LC_NUMERIC
LC_TIME

The corresponding integer values would need to be determined form local.h during PHP compile time.

The invalid value -1 is also used as a category in some parts of the test ext/gettext/tests/44938.phpt:

var_dump(dcgettext($overflown, $msgid, -1));
var_dump(dcgettext($domain, $overflown, -1));
var_dump(dcngettext($overflown, $msgid, $msgid, -1, -1));
var_dump(dcngettext($domain, $overflown, $msgid, -1, -1));
var_dump(dcngettext($domain, $msgid, $overflown, -1, -1));

For this test there's no crash, I think because it first detects the overly long argument it tests for, so the atual gettext library call is never done.

Note also, that only the category argument -1 is invalid, not e.g. the -1 in

var_dump(ngettext($overflown, $msgid, -1));

or the first of the two -1 in

var_dump(dcngettext($domain, $msgid, $overflown, -1, -1));

Note that IMHO this is not a duplicate of

https://bugs.php.net/bug.php?id=47663



Patches

bug70542 (last revision 2015-10-24 05:26 UTC by kalle@php.net)
gettext-tests-valid-categories (last revision 2015-09-24 13:52 UTC by rainer dot jung at kippdata dot de)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-09-22 04:05 UTC] laruence@php.net
I can not reproduce this on Linux
 [2015-09-22 05:57 UTC] rainer dot jung at kippdata dot de
Sure, it is *not* happening on Linux. That's why I assigned it to Solaris and wrote "Linux has special provisions in the source code for invalid category values, but on Solaris invalid values make the test crash."
 [2015-09-23 06:33 UTC] laruence@php.net
do you have a suggest patch?
 [2015-09-24 13:51 UTC] rainer dot jung at kippdata dot de
I'll now attach a yet untested but suggested patch. First I started adding the LC_ constants to gettext but then I noticed, that those constants already exist in core PHP, so the patch is just a small change to the tests.

I currently run a fresh build and then will execute the full test suite and then report back, whether the patch actually did work for me.
 [2015-09-24 16:24 UTC] rainer dot jung at kippdata dot de
Confirmed, the patch works.
 [2015-10-24 05:01 UTC] kalle@php.net
I think we should fix this at the source itself, while also fixing the tests on the side. Since we now know that the underlying library may segfault if we pass an unexpected value from the outside to it, we may as well have a check before calling dcngettext() (or any other function using a $category parameter in ext/gettext) to ensure its not possible to cause a crash.
 [2015-10-24 05:24 UTC] kalle@php.net
Running some tests shows me that under Windows, $category with -1 is perfectly fine, meaning it could most likely be a Solaris library port issue.

I have cooked up a patch, maybe you could give it a try (patched against git master) and see if it works.
 [2015-10-24 05:26 UTC] kalle@php.net
The following patch has been added/updated:

Patch Name: bug70542
Revision:   1445664412
URL:        https://bugs.php.net/patch-display.php?bug=70542&patch=bug70542&revision=1445664412
 [2015-10-24 05:37 UTC] kalle@php.net
-Status: Open +Status: Feedback
 [2015-11-01 04:22 UTC] php-bugs at lists dot php dot net
No feedback was provided. The bug is being suspended because
we assume that you are no longer experiencing the problem.
If this is not the case and you are able to provide the
information that was requested earlier, please do so and
change the status of the bug back to "Re-Opened". Thank you.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Tue Aug 20 09:01:28 2019 UTC