Segmentation fault (core) with 3.10.0 on strtr() on some chars if passed array exeeds 64 items

[keepxtreme]

Hi everyone,

starting with the update 3.9.1 to 3.10.0 dokuwiki starts crashing on Mainpage (doku.php). The hhvm/error.log only reports segmentation fault: core dumped and refers to a tmp-log:

PHP Stacktrace:
#0 utf8_deaccent(:, -1) called at [/var/www/service/wiki/inc/pageutils.php:132]
#1 cleanID(:) called at [/var/www/service/wiki/inc/pageutils.php:294]
#2 wikiFN(:, ) called at [/var/www/service/wiki/inc/pageutils.php:549]
#3 resolve_pageid(, :, ) called at [/var/www/service/wiki/inc/template.php:941]
#4 tpl_youarehere() called at [/var/www/service/wiki/lib/tpl/dokuwiki/tpl_header.php:84]
#5 include(/var/www/service/wiki/lib/tpl/dokuwiki/tpl_header.php) called at [/var/www/service/wiki/lib/tpl/dokuwiki/main.php:33]
#6 include(/var/www/service/wiki/lib/tpl/dokuwiki/main.php) called at [/var/www/service/wiki/inc/actions.php:196]
#7 act_dispatch() called at [/var/www/service/wiki/doku.php:124]

I tested the function utf8_deaccent and it crashes if only a ":" string is passed using the specified array:

if(!function_exists('utf8_deaccent')){
    /**
     * Replace accented UTF-8 characters by unaccented ASCII-7 equivalents
     *
     * Use the optional parameter to just deaccent lower ($case = -1) or upper ($case = 1)
     * letters. Default is to deaccent both cases ($case = 0)
     *
     * @author Andreas Gohr <andi@splitbrain.org>
     *
     * @param string $string
     * @param int $case
     * @return string
     */
    function utf8_deaccent($string,$case=0){
        if($case <= 0){
            global $UTF8_LOWER_ACCENTS;
            $string = strtr($string,$UTF8_LOWER_ACCENTS);
        }
        if($case >= 0){
            global $UTF8_UPPER_ACCENTS;
            $string = strtr($string,$UTF8_UPPER_ACCENTS);
        }
        return $string;
    }
}

[keepxtreme]

ok, I digged deeper and it seems like strst() causes a seg fault if the passed array exeeds 64 pairs

[matzekuh]

I tried to reproduce the problem on 3v4l. I issued strtr(":",$UTF8_UPPER_ACCENTS).
$UTF8_UPPER_ACCENTS is taken from the dokuwiki repo: https://github.com/splitbrain/dokuwiki/blob/master/inc/utf8.php

Case 1: $UTF8_UPPER_ACCENTS with 64 entries not containing ":".
(https://3v4l.org/BnTDC) works fine.

Case 2: $UTF8_UPPER_ACCENTS with 65 entries not containing ":".
(https://3v4l.org/HMLUV) hhvm-3.10.0 exited with code 137
Case 2a: $UTF8_UPPER_ACCENTS with 65 entries not containing ":" but containing "!".
(https://3v4l.org/XEYGC) hhvm-3.10.0 works fine

Case 3: $UTF8_UPPER_ACCENTS with 65 entries containing ":".
(https://3v4l.org/prZbb) works fine.

For every case the example works in hhvm-3.9.1.

The crash seems to depend on the given replacement array. Unfortunately I was not able to find some system behind it until now.

[jwatzman]

Interesting -- sounds like you found a repro case on 3v4l, which is really useful. Someone will take a look at this when we get a chance, thanks!

[keepxtreme]

3.10.0 crashs in Drupals Admin Menu module in the function admin_menu_tree(): http://cgit.drupalcode.org/admin_menu/tree/admin_menu.inc
This is as well related to strtr() as commenting out the line containing the function brings the function back to work. However, as this module is enabled on a whole bunch of drupal sites I usume it as critical to get this fixed

[SiebelsTim]

I did not do a bisect, but I'm fairly certain that b94ac59 is the commit that introduced the issue.

My reasoning is, that commit introduced a new path for strtr when the array has more than 64 elements (kBitsPerQword == 64).

Furthermore I know that WuManberReplacement is initialized with a minLen of 2 which results in a unsigned overflow here, because your source string is one character long.
You will see no segfault when you increase your source string length.

I've got to run so I can't look at this right now. Maybe later today or in a few days.

e: That algorithm probably only looks at the string length inside the array and forgets about the source string. I guess you can reproduce this bug with having >64 array elements, and every element being longer than the source string. Will try to confirm later.

[jwatzman]

@SiebelsTim awesome, thanks for the investigation! Does seem likely, I'll confirm later if you haven't gotten around to it yet. I've poked the author on the original PR to take a look here; if they don't have a satisfactory fix shortly, we can revert this.