Make WordPress Core

Opened 10 years ago

Closed 9 years ago

#30802 closed defect (bug) (fixed)

PHP Segfault with SSH2 prevents General Settings page loading

Reported by: hyoseop's profile hyoseop Owned by: dd32's profile dd32
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.1
Component: Filesystem API Keywords: needs-patch fixed-major
Focuses: Cc:

Description

After upgrading to 4.1, I can't open General menu in Settings. When I click General menu, the server don't reply. The file of this menu is options-general.php. And below is the path of tracing a reason.

wp-admin/options-general.php

  • 356 : wp_can_install_language_pack()

wp-admin/include/translation-install.php

  • 233 : fs_connect( array( WP_CONTENT_DIR, WP_LANG_DIR ) )

wp-admin/include/class-wp-upgrader.php

  • 193 : $wp_filesystem->find_folder($dir)

wp-admin/include/class-wp-filesystem-base.php

  • 273 : search_for_folder($folder)
  • 293 : trailingslashit($this->cwd()); ====> $this->cwd() is defined to return false always at 525 line.
  • 1661 : untrailingslashit( $string ) . '/';
  • 1667 : rtrim( $string, '/
    ' ); ====> $string isn't string. It is boolean value 'false'.

I can't find where rtrim is defined. But it is clear rtrim must have a string variable as first input. In this case, rtrim gets boolean value 'false' as $string. This makes server blocked. If I remove this function(This means skipping this.), the general settings screen is shown properly. And I don't know why a boolean value is given as a first string variable.

Why rtrim gets a boolean value as a first string variable?
If it is a root reason, how do I fix this problem?

If you want to see the problem, visit here.
http://칭기즈칸.한국/
And general settings path is below.
http://칭기즈칸.한국/wp-admin/options-general.php

Change History (20)

#1 @hyoseop
10 years ago

1661:untrailingslashit and 1667:rtrimg is defined in wp-includes/formatting.php

#2 @ocean90
10 years ago

  • Component changed from General to Filesystem API

#3 @ocean90
10 years ago

  • Keywords reporter-feedback added

Hello hyoseop,

can you please provide some infos about your server config like PHP version and which filesystem API is in use? If you do not know this you can use Core Control with the Filesystem Module.

#4 @SergeyBiryukov
10 years ago

This might help:

If you're getting weird 502's and/or half missing general settings screen, then your server has an old version of libssh, and is using ssh for filesystem connections. On the general-settings screen, 4.1 checks the filesystem to see if it is writable by normal means so it knows whether or not to display all available languages, which are now auto-installed/updated.

https://wordpress.org/support/topic/read-this-first-wordpress-41-master-list?replies=5#post-6347399

Last edited 10 years ago by SergeyBiryukov (previous) (diff)

#5 @nacin
10 years ago

  • Milestone changed from Awaiting Review to 4.1.1

Moving to 4.1.1 while we wait for feedback from hyoseop.

#6 @johnbillion
10 years ago

Any update for us hyoseop? We've not had any other reports of this so it's looking like it's probably a localised issue.

#7 @bravadomizzou
10 years ago

I work with several sites running Wordpress, I have had it happen on 3 sites that have updated to 4.1

#8 @hyoseop
10 years ago

Dear all replier,

Sorry for my late answer and thank you all. I had focused on my job, and I haven't recognized that it has been so long time.

@ocean90,
I installed Core Control, and I checked filesystem module. But nothing is changed.
What should I do after install Core control??

@Sergey Biryukov
According to your advice, I got to know that the old libssh is the problem. But I don't know what I have to do.
I use ubuntu as server operator system. And I installed libssh-4. But nothing is changed.
What do I have to do?

#9 @mc_teo
10 years ago

Hey All,

I just updated a WordPress installation to 4.1 and have the same problem: general settings won't open, and it won't allow me to delete plugins. Both get the same error.

I searched around, and in the Apache error log, there is the following line, multiple times per page request:

[Mon Feb 02 15:08:55 2015] [error] [client <ip>] PHP Warning:  stream_set_timeout(): No support for ssh2 stream timeout. Please recompile with libssh2 &gt;= 1.2.9 in /home/<user>/public_html/wp-admin/includes/class-wp-filesystem-ssh2.php on line 139, referer: http://<host>/wp-admin/

The line referred to is: (class-wp-filesystem-ssh2.php:139)

stream_set_timeout( $stream, FS_TIMEOUT );

The version of libssh2 I had installed (latest version from Ubuntu 10.04 repos) was about 1.1.0, so I built the latest libssh2 version (1.4.3), and built the latest php-ssh2 extension (0.12) (using phpize if it matters), and reloaded apache. It now shows up in phpinfo() that php-ssh2 was built with 1.4.3, but it didn't change anything with the error. I don't know if its a case of something else that needs to be changed to force WordPress to use this updated extension, or what.

It should be noted that I hardcoded the credentials in wp_config.php

define('FTP_PUBKEY', '/home/<user>/.ssh/id_rsa.pub');
define('FTP_PRIKEY', '/home/<user>/.ssh/id_rsa');
define('FTP_USER', '<user>');
define('FTP_PASS', '');
define('FTP_HOST', 'localhost');

On how I built the php-ssh2 extension; I just followed http://php.net/manual/en/install.pecl.phpize.php, but now I notice in phpinfo(), there is a PWD environmental variable set to the path that I built php-ssh2. Shouldn't this be the path the .php file is in?

#10 @dd32
10 years ago

Can someone please enable WP_DEBUG or check your servers PHP error logs to let us know what the error being hit is?

The PHP Warning of stream_set_timeout() is somewhat related, but isn't the cause of the issue, and there's likely a PHP Fatal error coming from something else (It could even just be a timeout or memory limit exceeded error).

The only changes between 4.0 and 4.1 related to the SSH transport was fixing the move() method (Diff) and the only changes between 3.7 and 4.0 were related to the getchmod() method (Diff).

It seems incredibly strange that Plugin/Theme/Core update functionality would be working previously if it's breaking as suggested here on 4.1, either way, the exact error message should be able to shed some light on it.

#11 @dd32
10 years ago

  • Keywords close added
  • Milestone changed from 4.1.1 to 4.1.2

#12 @SergeyBiryukov
10 years ago

  • Keywords needs-patch added; reporter-feedback close removed

See #31554 for a possible explanation.

#13 @jobst
10 years ago

Hi hyoseop

This issue ONLY occurs when the following two conditions are met

  • You have specified the FTP setting in wp-config.php
  • You have NO languages directory below /PATH_TO_WORDPRESS/wp-content/

See #31554 for deeper explanation.

MY RESEARCH INTO THIS:

It took me some tracing to find that the error was created in /PATH_TO_WP_INSTALL/wp-admin/includes/class-wp-upgrader.php in the function fs_connect in the switch statement "default" case. I included a debug statement in that function right at he beginning to see what path generated this error:

 error_log(" FS_CONNECT: ".print_r($directories,1));

which gave me this output on fail:

[07-Mar-2015 06:44:40 UTC]  FS_CONNECT: Array
(
    [0] => /PATHDELETED/wp-content
    [1] => /PATHDELETED/wp-content/languages
)

while the APACHE /var/log/httpd/error_log displayed this:

[Sat Mar 07 17:44:40 2015] [notice] child pid 578 exit signal Segmentation fault (11)

It took me some time to get to the real problem:

  • When the function "fs_connect" (see a couple of lines above) is called, it calls a function find_folder($dir) where $dir is (when the seg fault is called) "/PATHDELETED/wp-content/languages".
  • find_folder resides in /PATHDELETED/wp-admin/includes/class-wp-filesystem-base.php on line 222. If this function cannot find the folder after some 30 lines of code tried to find it, the function employs another function called "seach_for_folder()" which is in the same file on line 291.
  • Right at the start of that function "seach_for_folder()" there is an if statement checking whether $base is empty or '.', if either it sets $base = railingslashit($this->cwd()); which actually is the bug.
  • all trailingslashit does appending a "/" to the end of the string returned by $this->cwd() - however $this->cwd() returns a string that includes a "\n", this crashes wordpress with a segfault so in fact the return string has a newline character in between the string and the /

FAULTY EXAMPLE

if ( empty( $base ) || '.' == $base )
  $base = trailingslashit($this->cwd());
error_log("BASE: ".$base);

BASE: /home/ssh_user_name
/

CHANGED CODE TO FIX THE BUG:

if ( empty( $base ) || '.' == $base )
{
  $tmp=$this->cwd();
  $tmp=preg_replace("/[\r\n\m\t]/","",$tmp);
  $base = trailingslashit($tmp);
}
echo "BASE: $base"

BASE: /home/ssh_user_name/

The last example does not SEGFAULT the system.

Jobst

Last edited 10 years ago by ocean90 (previous) (diff)

#14 @ocean90
10 years ago

#31554 was marked as a duplicate.

#15 @dd32
10 years ago

  • Summary changed from General settings menu isn't opened. to PHP Segfault with SSH2 prevents General Settings page loading

Duplicated the Segfault under PHP 5.3.3.

The bug has been present for quite some time, but has only just now been triggered.
Most SSH setups will have the full absolute path available, so it never actually triggered the cwd() method, since all the directories existed. This segfault can be worked around by simply creating the wp-content/langauges/ directory.

All commands that return output are doing so as such: $output\n, turns out that if you pass a path of /something\n/something you can cause a segfault in PHP. I haven't tracked down the exact method that segfaults with it, but i vaguely recall seeing a PHP bug report that got fixed at some point.

#16 @dd32
10 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 31686:

SSH2 Upgrade transport: Trim the trailing newline character from the ouput of pwd to avoid a PHP Segfault.

Fixes #30802 for trunk.

#17 @dd32
10 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.1 consideration.

#18 @jobst
10 years ago

I am not sure why the installer of wordpress does not simply create "wp-content/languages".

If you look at the 100's of lines of code (and the CPU cycles wasted) to "find" the directory it would be IMHO a very good idea to create the directory right from the install onwards.

If you consider that most security conscious people would have the owner of the wordpress tree be a different UID of the user running apache then just consider how many directries find_folder and search_for_folder go through to find "languages", which even includes the home directory of the OWNER (never ever should this be touched) - only at the end not to find it at all!

I am not sure whether this is a security problem as well considering while this is runnning it looks at some files that are outside the wordpress tree ...

Please Wordpress, consider making the creation of "wp-content/langauages/" part of the installer!

Jobst

Last edited 10 years ago by jobst (previous) (diff)

#19 @ocean90
10 years ago

#31167 was marked as a duplicate.

#20 @pento
9 years ago

  • Milestone changed from 4.1.2 to 4.2
  • Resolution set to fixed
  • Status changed from reopened to closed

With 4.2 being just around the corner, we're going to skip this for 4.1.2.

Note: See TracTickets for help on using tickets.