Make WordPress Core

Opened 11 years ago

Closed 9 years ago

Last modified 8 years ago

#28633 closed enhancement (fixed)

Generate better random numbers

Reported by: sarciszewski's profile sarciszewski Owned by: dd32's profile dd32
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Security Keywords: needs-testing has-patch early
Focuses: Cc:

Description

Never used Trac/SVN at all, so I apologize if I'm doing this wrong. I want to submit what a github user would call a pull request. Although this is classified as security, it's not exactly a critical 0day vulnerability I found or anything.

Attachments (15)

28633.patch (5.5 KB) - added by sarciszewski 10 years ago.
Bugfix
28633.2.patch (7.2 KB) - added by sarciszewski 10 years ago.
Updated patch
28633.3.patch (9.4 KB) - added by sarciszewski 10 years ago.
Now with Windows + 5.2 support
28633.4.patch (9.5 KB) - added by sarciszewski 10 years ago.
Edited the patch file manually because git://develop.git.wordpress.org is not working atm
28633.5.patch (8.5 KB) - added by sarciszewski 10 years ago.
Figured the git problem out, previous patch didn't work, so I remade it.
28633_alt.patch (8.8 KB) - added by sarciszewski 10 years ago.
Alternative, based on https://github.com/resonantcore/lib/blob/develop/src/Secure.php which I have tested extensively.
28633.6.patch (10.6 KB) - added by sarciszewski 10 years ago.
More paranoid return checking, use a binary-safe strlen() and substr()
28633_with_md5.patch (10.0 KB) - added by sarciszewski 10 years ago.
Same as previous patch but without replacing the md5(time()) hack
28633.6.1.patch (9.8 KB) - added by yantchi 10 years ago.
Patch 28633.6 with fixed syntax errors (few missed closing parentheses and try-catch syntax)
28633.7.patch (11.4 KB) - added by sarciszewski 10 years ago.
@Otto42 - You're absolutely correct. Also, there's a bug in mcrypt on Windows for PHP < 5.3.7 where it always produces insufficient entropy.
28633.8.diff (6.1 KB) - added by dd32 10 years ago.
28633.9.diff (5.8 KB) - added by dd32 10 years ago.
28633.10.patch (33.9 KB) - added by sarciszewski 9 years ago.
Use paragonie/random_compat
28633.11.patch (34.1 KB) - added by sarciszewski 9 years ago.
Return after random_int()
28633.diff (2.5 KB) - added by dd32 9 years ago.

Download all attachments as: .zip

Change History (89)

#1 @johnbillion
11 years ago

Thanks for your interest in WordPress, sarciszewski!

If your issue is a security enhancement rather than a vulnerability, then go ahead and post it here.

If you think it might be a vulnerability, or if you're not sure, then you should email your report to security@wordpress.org rather than posting it here.

Thank you

#2 @sarciszewski
11 years ago

Okay, SVN is giving me a headache. Here's the pull request on github. I give up on trying to figure out how to work SVN/Trac.

https://github.com/WordPress/WordPress/pull/84

Basically, this adds a wp_secure_rand() function that takes 1 parameter (the number of bytes desired) and returns a raw binary stream. This should be use in place of rand(), mt_rand(), uniqid(), etc in places where the unpredictability of the output matters to security. (e.g. password resets)

I've also patched wp_rand() to call this function internally, so that all random numbers generated by wp_rand() benefit from this greater security. Where possible, I tried to make changes seamless to developers.

#4 @sarciszewski
11 years ago

I apologize for the ms.php and schema.php patch being literally every line in the file. I think there's a whitespace issue somewhere. The only lines that should be changed are the ones with the string wp_secure_rand in my version.

#5 follow-up: @johnbillion
11 years ago

  • Keywords needs-patch needs-testing added

You can get GitHub to hide whitespace changes by tacking the w=1 parameter onto the URL: https://github.com/sarciszewski/WordPress/commit/22f587ea4b22ed0e4473a52832b4fbd5ecaa58c1?w=1

Regardless, the patch will need re-doing without the whitespace changes.

#6 @helen
10 years ago

  • Version changed from trunk to 3.9

#7 @sarciszewski
10 years ago

  • Severity changed from normal to major

The patch I provided resolves CVE-2014-6412 -- I would appreciate it if this was given a second look, esp. by the security team.

#8 @nacin
10 years ago

I'm just seeing this. If you've requested a CVE identifier, I'm thinking maybe this should have been reported privately.

http://make.wordpress.org/core/handbook/reporting-security-vulnerabilities/

#9 @sarciszewski
10 years ago

I requested a CVE identifier this week, the patch was submitted in June. I've reported the avenue to exploitation to your security team.

The actual exploitation (which I will not discuss publicly) is not trivial, and I don't expect most script kiddies are capable of doing so.

EDIT: The reason is, I didn't realize until this past Friday how it could be leveraged to abuse a WordPress installation. While I understand that security holes are sensitive matters, the fact remains you had the patch for months and nobody did anything with it.

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

#10 @sarciszewski
10 years ago

As a follow-up, I feel that I should mention that I don't intend on dropping exploit code on Full Disclosure or anything reckless like that. If the patch I provided isn't sufficient for you, feel free to take your time to fix the problem and make sure the fix is correct. Just, please, keep me updated on where everyone is.

#11 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
10 years ago

Replying to johnbillion:

Regardless, the patch will need re-doing without the whitespace changes.

I think line ending format difference is the issue.

#12 in reply to: ↑ 11 ; follow-up: @sarciszewski
10 years ago

Replying to SergeyBiryukov:

Replying to johnbillion:

Regardless, the patch will need re-doing without the whitespace changes.

I think line ending format difference is the issue.

If the github repo were authoritative, patching this would be a lot smoother on my end. But I don't have the time or patience to learn SVN right now.

#13 in reply to: ↑ 12 @SergeyBiryukov
10 years ago

Replying to sarciszewski:

If the github repo were authoritative, patching this would be a lot smoother on my end. But I don't have the time or patience to learn SVN right now.

This might help:
http://scribu.net/wordpress/svn-patches-from-git.html
http://scribu.net/wordpress/contributing-to-wordpress-using-github.html

#14 @nacin
10 years ago

This is now being handled by the security team and with sarciszewski privately. No need for further conversations here, thanks.

#15 @sarciszewski
10 years ago

  • Keywords needs-patch removed

@sarciszewski
10 years ago

Bugfix

#16 @sarciszewski
10 years ago

  • Keywords has-patch added

@sarciszewski
10 years ago

Updated patch

#17 @sarciszewski
10 years ago

Current patch does the following:

  • Adds functions:
    • wp_random_bytes(int $number)
    • wp_random_positive_int()
    • wp_secure_rand(int $min, int $max)
  • Does not patch wp_rand() like the previous patch did.
  • Updates wp_generate_password() to use wp_secure_rand() instead.
  • Adds a unit test in tests/phpunit/tests/functions.php

I'd like to request this gets bumped up for evaluation in 4.1 or 4.1.1

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

@sarciszewski
10 years ago

Now with Windows + 5.2 support

#19 @sarciszewski
10 years ago

The latest patch should now work on incredibly ancient versions of Windows. (a.k.a. 5.2)

x Supports unsupported PHP versions
x Has unit tests

This should now be fully compatible with your requirements.

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

#21 @sarciszewski
10 years ago

  • Version changed from 3.9 to trunk

Head's up, sent this to Full Disclosure.

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

#23 @sarciszewski
10 years ago

From CodeIgniter 3.0-dev:

is_php('5.4') && stream_set_chunk_size($fp, $length);

I think this can resolve the issue Stefan Esser pointed to.

@sarciszewski
10 years ago

Edited the patch file manually because git://develop.git.wordpress.org is not working atm

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

@sarciszewski
10 years ago

Figured the git problem out, previous patch didn't work, so I remade it.

@sarciszewski
10 years ago

Alternative, based on https://github.com/resonantcore/lib/blob/develop/src/Secure.php which I have tested extensively.

#25 @nacin
10 years ago

This coming in as a public bug report immediately triggered alarm bells. When this happens, and it does sometimes, the security team mobilizes to see whether the report is indeed a vulnerability. It was pretty quickly determined that while there may be something that could be improved here, there was no smoking gun here. At some point, @sarciszewski started emailing me directly, rather than the security team or by posting here. I never saw these due to aggressive email filters. I get a lot of email and don't have time to read lower priority stuff in a timely manner; there's a reason why it's a security team and not a nacin team.

@sarciszewski then posted this to FD: http://seclists.org/fulldisclosure/2015/Feb/42. I'm paraphrasing an old movie here: it's not what was done, it's how it was done. I deal with a lot of tiring stuff every day, and the tone wasn't all that necessary to get the results he wanted. If he really does think WordPress has a critical issue, then the facts would ideally speak for themselves. I sent a tweet in reply to @sarciszewski that vaguely tried to convey my feelings about the tone (not content). That then resulted in a lot of random people replying at me in shall we say less than complimentary ways, because they found the tweet through Reddit, so I deleted it. It's archived on Reddit if you wish to read it.

Like many complicated security-related issues in WordPress, the original code here was written far before I got involved on the project. Someone else mentioned on Reddit some previous circumstances where I ended up somehow being the public face for WordPress.com delivering their cookies in the clear (1), and also WordPress not having user cookies backed by a server-side session (2). For the first part, yeah, it was a bug, and they addressed all aspects of it in under a week. (Also note, I don't work at Automattic or at WordPress.com, and never have.) That second part is a known issue going back to the b2/cafelog project WordPress was forked from 12 years ago. It wasn't something that could be changed overnight. Everyone of course knew it was an issue. Once it became clear we did have a viable and performant solution for it, we shipped it.

There was a reason WordPress cookies only last for 14 days even when "Remember me" is checked, and why of course we enforce cookie expiration times as part of our auth cookie hash. Now that we have sessions, it's perhaps time to extend that beyond 14 days, but with most sites not having SSL, that's still not exactly ideal either. Security and usability always have a tradeoff. And, hey, look, I sure as hell didn't write it that way more than a decade ago.

In the course of my research on this CSPRNG bug report, I found that the original code here was written in 2008 in response to a report by Stefan Esser, a well-known security researcher who is intimately familiar with PHP, and has reported a few issues to us over the years. I couldn't locate any conversations on our end, but I did some research and was comfortable with the solution we had as a result of that work, as hacky as it was. Some of Stefan's comments on Twitter have supported that sentiment, though I've asked more specifically whether he also thinks that's true.

For the record: I think it would be nice to make an adjustment here. This is precisely the kind of security debate that a maintainer loves to see happen out in the open, in a healthy and positive manner, because it only means the software will get better.

@sarciszewski
10 years ago

More paranoid return checking, use a binary-safe strlen() and substr()

#26 @jeremyfelt
10 years ago

In schema.php, $hostname = substr( md5( time() ), 0, 6 ) . '.' . $domain; // Very random hostname! is used as a random enough hostname only to check that wildcard DNS is supported for subdomain multisite installs. We can leave it out of any improvements.

#27 follow-up: @sarciszewski
10 years ago

We can leave it out of any improvements.

I disagree strongly for one simple reason: Some developers can and will refer to the WordPress core as a source of authority and inspiration. If we expose an insecure method as "very random", it may lead to bad habits propagating into the next generation.

I learned how to use MySQL from PHP by reading the source code in nulled copies of Invision Power Board circa 2002.

#28 in reply to: ↑ 27 @jeremyfelt
10 years ago

Replying to sarciszewski:

If we expose an insecure method as "very random", it may lead to bad habits propagating into the next generation.

I learned how to use MySQL from PHP by reading the source code in nulled copies of Invision Power Board circa 2002.

Absolutely. Improved documentation to clarify code is always good. The "very random" in this case is—or I read it as— tongue in cheek in the context of the surrounding code, though it could be updated.

The md5() approach itself doesn't really need to be made more complex as security is not a factor here.

#29 follow-up: @sarciszewski
10 years ago

The md5() approach itself doesn't really need to be made more complex as security is not a factor here.

Are you saying to revert it, or "since you made it secure random anyway, leave it, but it wasn't necessary"?

Taking 3 random bytes and converting it to hex is actually simpler than taking a cryptographic hash of the current time, then grabbing the first 6 hexits from it, since the secure random is abstracted away.

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

#30 in reply to: ↑ 29 @jeremyfelt
10 years ago

Replying to sarciszewski:

Are you saying to revert it, or "since you made it secure random anyway, leave it, but it wasn't necessary"?

Let's leave it out of any patch on this ticket. We can address any simplification or documentation of that segment elsewhere.

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

@sarciszewski
10 years ago

Same as previous patch but without replacing the md5(time()) hack

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

@yantchi
10 years ago

Patch 28633.6 with fixed syntax errors (few missed closing parentheses and try-catch syntax)

#33 @Otto42
10 years ago

I think you might need a php version check on the if (function_exists('mcrypt_create_iv')) line as well. On a Windows machine running 5.2.x, where the mcrypt extension is also enabled, a call made using MCRYPT_DEV_URANDOM will cause a "Cannot open source device" warning. This warning will prevent logins if display_errors is enabled (which it sadly is by default on most hosts).

Alternatively, error suppression by using @mcrypt_create_iv() would probably work just as well.

@sarciszewski
10 years ago

@Otto42 - You're absolutely correct. Also, there's a bug in mcrypt on Windows for PHP < 5.3.7 where it always produces insufficient entropy.

#34 @sarciszewski
10 years ago

@yantchi - Thanks for catching those, I don't know what I was thinking :)

@dd32
10 years ago

#35 @dd32
10 years ago

I've spent some time looking into this, and as a hardening-only patch, this seems like it's going in a good direction. I'm approaching this explicitly as a hardening-only patch as I believe wp_rand() we have in WordPress is already safe from prediction (and related) attacks, but that it was also written when the latest version of PHP available was 5.2.8, so did not have most of these available to it.

Here's a rundown of the currently available sources that we can use:

  • Mcrypt
    • If the extension is available
    • PHP 5.3.7+ only for Windows (uses native Crypto Library)
  • /dev/urandom
    • If *nix and readable
    • may eat entropy in PHP < 5.3.3 (as it'll use read-ahead buffering)
    • not available on Windows
  • OpenSSL
    • If the extension available
    • PHP 5.3.4+ only for Windows, as it's interactive-display random seed uses lots of processing time (30-60s+) on a non-interactive screen
    • Uses Windows native Crypto API in PHP 5.4+ (might be a better minimum for windows)
    • May use a non-strong crypto source on some "rare" systems (see $crypto_strong param)
    • Also worth noting OpenSSL has a myriad of random generators, I believe at least one of these leverages /dev/random (or urandom/srandom) on supported platforms, but I'm unable to determine what platforms/OpenSSL versions at a glance
  • Windows specific: CAPICOM
    • Deprecated; For Windows 2008 and lower (XP, Visa, 7, 2003, 2008)
    • Available when the optional package is installed
    • Haven't tested this, seems like it can have permissions issues
  • Windows specific: DOTNET API
    • Supported in Windows 2008 R2 and greater with .NET 1.0 or greater bindings enabled
    • Haven't tested this, seems like it can have permissions issues
    • Reports of it not working reliably at all in PHP due to incompatible types

It seems that randomness support on Windows servers is not-great in older versions of PHP, and we should potentially ditch windows-specific external API's and leverage OpenSSL/Mcrypt instead as they work reliably with the native Windows crypto API's in PHP 5.3.7/5.4+.

The simple approach we can take here, assuming that our existing wp_rand() is not insecure, is to simply beef up wp_rand() to leverage these alternative random sources when available.
This also leads me to questioning if we should support /dev/urandom which although is available on many hosting environments, is also less and less useful as OpenSSL or Mcrypt are available on more installs. Although i'm not sure of the penetration of Mcrypt, OpenSSL is very commonly supported. The code to use urandom is lightweight and straight-forward however, so I don't see a significant reason not to include it.

So, I've attached two patches

  • 28633.8.diff This is my WIP patch against the ones suggested here and was based off of 28633_with_md5.patch
    • PHP syntax issues corrected
    • Windows version checks added
    • uses our mbstring_binary_safe_encoding() function to avoid the need for wp_binsafe_strlen()
    • rewrites the urandom handling in wp_random_bytes() to be more explicit and cleaner
    • fixes wp_secure_rand() to return inclusive values between $min and $max while making the code easier to read (It can also now handle wp_secure_rand(1, 2) which it couldn't previously)
    • a bunch of other formatting and code cleanliness issues
  • 28633.9.diff however is an initial rewrite into what I think we should aim for
    • it renames the functions (although I'm really not sold on the names I've chosen at all, just wanted to remove the secure keyword)
    • moves towards ditching wp_secure_rand() and simply having wp_rand()
    • Drops the Windows-specific branches but leaves urandom in play.
    • In this scenario these functions would live elsewhere, i'm just including them in this file for ease of patch comparison

Thoughts? Suggestions?

@dd32
10 years ago

#36 @sarciszewski
10 years ago

I am rather fond of 28633.9.diff personally. Avoiding wp_binsafe_strlen() is a good idea too :)

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


10 years ago

#39 @masakielastic
10 years ago

It is probable that random_bytes and random_int will be introduced in PHP 7.0. The vote will be finished at March 28th.

https://wiki.php.net/rfc/easy_userland_csprng

#40 @dd32
10 years ago

  • Keywords early added
  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from major to normal

I'd prefer to land these changes at the start of a cycle, to allow for full testing by everyone and to surface any issues from reliance upon any of the functions.

Based on the PHP7 RFC, which looks like it'll succeed, we should change the direction of the patch to simply provide a compat layer for the PHP7 function signatures instead, which will benefit us in the long term.

The changes:

  • wp_external_random_bytes() -> random_bytes() not sure if the byte length changes, or if it's a required param..
  • wp_external_rand() -> random_int() which also needs to support negative numbers, ie. random_int( -1000, -10 ) should work.
  • wp_external_random_positive_int() becomes an internal private function, only defined when the wrapper for random_int is, however it might not even be needed based on the negative number support for random_int()

#41 @iseulde
10 years ago

  • Version trunk deleted

#42 @sarciszewski
10 years ago

wp_external_random_positive_int() is there to generate a random number along a range. If you use bitshifting to get 0..1023 then add the minimum (-1000), that should be suitable for (-1000, -10).

(The result from wp_external_random_positive_int() will be discarded if it exceeds 990 in that case, and it will start over.)

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

#43 @sarciszewski
10 years ago

Hey, it's been a while.

I see that this still hasn't gotten merged. This might be a good thing.

I've put together a polyfill library for PHP7's random_int() and random_bytes() functions for PHP 5.x projects and (with WordPress in mind) I kept PHP 5.2 compatibility.

https://github.com/paragonie/random_compat

@dd32 If you would prefer to use an external library and have wp_external_random_bytes() just invoke it rather than try to manage this internally, that might make upgrading/patching any bugs that crop up in the future more sane. "Upgrade random_compat to 1.0.1" is probably an easier change to audit than a comprehensive rewrite of core functions.

#44 @sarciszewski
9 years ago

@dd32 random_compat should see a 1.0.0 release this weekend.

This ticket was mentioned in Slack in #core by miss_jwo. View the logs.


9 years ago

#46 @dd32
9 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to dd32
  • Status changed from new to reviewing

I'm marking for 4.4 for review purposes. I'll go through the libraries code and see if there's going to be any issues - at first glance It all looks pretty straight forward and compatible, so shouldn't be an issue.

#47 @sarciszewski
9 years ago

Sounds great! v1.0.0 has been tagged and released. Additionally, it should work all the way back on 5.2.4 on Windows without ext/mcrypt. :)

https://github.com/paragonie/random_compat/releases/tag/v1.0.0

This ticket was mentioned in Slack in #core by sarciszewski. View the logs.


9 years ago

@sarciszewski
9 years ago

Use paragonie/random_compat

@sarciszewski
9 years ago

Return after random_int()

#49 @dd32
9 years ago

Fun facts: wp_rand() only returns positive numbers (no matter the input), and it also doesn't care about the order of $min and $max.

@sarciszewski Since PHP's implementation of random_int() has changed in the last few days since PHP7-RC2 was released (They've switched from PHP warnings to Exceptions, which your compat library deals with), I'd like to test this against PHP7-RC3 which is due to be released at the end of the week. I'm having a hard time comparing the compat library and PHP7's behaviours to ensure we don't have any issues at release time.

#50 @sarciszewski
9 years ago

Testing versus RC3 makes sense to me.

#51 @helen
9 years ago

Just a note that RC3 is out now if somebody would like to test, so we don't lose momentum here. :)

@dd32
9 years ago

#52 @dd32
9 years ago

28633.diff is tested against PHP 5.6 & PHP7-RC3 (This patch doesn't include the random_compat library itself though, for easier review)

A few things to note

  • wp_rand() always returns positive numbers, even if a negative range is offered
  • wp_rand() accepts the parameters in either order
  • streamlined the try {} catch {} catch {} to avoid needing to use version comparisons

The only issue I noted in the compat library, is that PHP7's random_int() claims to accept Integers, but accepts numeric types (floats/numeric strings) happily, and it appears that wp_rand() probably does too. As such, https://github.com/paragonie/random_compat/compare/master...dd32:compat-types?expand=1 is my work-in-progress at allowing it.

I was delayed in testing this thanks to conferences & the PHP7 packages being delayed (I'm testing using Webtatic PHP7 packages for Centos/RHEL 6 (In case anyone wants to also verify my experience)

Version 0, edited 9 years ago by dd32 (next)

#53 @sarciszewski
9 years ago

Should it return abs( $val ); or absint( $val );?

Your patch for random_compat makes sense. I can't believe I overlooked that. As soon as you're ready, I'll merge it and tag version 1.0.2, which is what I'd propose to be included in WP 4.4.

Thanks a ton, Dion.

#54 @dd32
9 years ago

Should it return abs( $val ); or absint( $val );?

Since random_int() should be returning an int already, they're mostly the same, however for consistency you're right, we should use absval().

Your patch for random_compat makes sense. I can't believe I overlooked that. As soon as you're ready, I'll merge it and tag version 1.0.2, which is what I'd propose to be included in WP 4.4.

Thanks for the super-fast merge! I couldn't find any other edge-cases in my testing, so a pretty good job :) The PHP7-style exception handlers seem solid too.

#55 @jorbin
9 years ago

One minor thing at a glance is that there is no reference in src/wp-includes/compat.php to the version of PHP that random_int is introduced. I think it makes sense to include a reference so that years in the future, when this compat library is no longer needed, it can be removed.

#56 @sarciszewski
9 years ago

That's a good point. Who knows when 5.6 will stop being supported by WP? I for one might be long gone when hosting companies finally get their act together and stay up-to-date.

Last edited 9 years ago by sarciszewski (previous) (diff)

#57 @dd32
9 years ago

After asking for more reviews of the code here, the only issue that's been raised, is the following use-case:

$val = wp_rand(); // (int) 0

This is because $min and $max are 0 by default, so.. it makes sense, but wp_rand() decides that this should instead be interpreted as a maximum of 4294967295. I think we can/should just use 4294967295 when $max = 0

#58 @dd32
9 years ago

In 34922:

Use PHP7's random_int() CSPRNG functionality in wp_rand() with a fallback to the random_compat library for PHP 5.x.
random_compat offers a set of compatible functions for older versions of PHP, filling in the gap by using other PHP extensions when available.
We still include our existing wp_rand() functionality as a fallback for when no proper CSPRNG exists on the system.

Props sarciszewski
See #28633

#59 @dd32
9 years ago

Lets see if anything breaks :)

A few thoughts:

#60 @dd32
9 years ago

In 34924:

Revert [34922] pending PHP 5.2 compatibility.
See #28633

#61 @dd32
9 years ago

Unfortunately we've run into a PHP 5.2 compatibility issue:
https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/84210246

Fatal error: Class Error contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Throwable::getPrevious) in src/wp-includes/random_compat/error_polyfill.php on line 48

It looks like this was caused by https://github.com/paragonie/random_compat/commit/90da7f3c0f4e92a1c44871de038e801d764ccc93

I've reverted it for now as I can't debug it properly on a PHP 5.2 system right this second.

#63 @dd32
9 years ago

@sarciszewski
Here's PHP parsing the committed code: https://3v4l.org/ei523
And here's PHP parsing the code with those method declarations removed: https://3v4l.org/Qsse1

No output is good, so it looks like removing those declarations fixes it, and is compatible with all PHP5 releases.
I'll recommit shortly with that change.

#64 @dd32
9 years ago

In 34981:

Use PHP7's random_int() CSPRNG functionality in wp_rand() with a fallback to the random_compat library for PHP 5.x.
random_compat offers a set of compatible functions for older versions of PHP, filling in the gap by using other PHP extensions when available.
We still include our existing wp_rand() functionality as a fallback for when no proper CSPRNG exists on the system.

Take Two, this was previously committed in [34922] but had an issue on PHP 5.2 which sarciszewski has now resolved.

Props sarciszewski
See #28633

#65 @SergeyBiryukov
9 years ago

In 34986:

Fix typo in wp_rand() docs.

See #28633.

#66 @SergeyBiryukov
9 years ago

#25816 was marked as a duplicate.

#67 @wonderboymusic
9 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

Calling it fixed. Reopen if the sky is falling.

#68 @dd32
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#69 @dd32
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 35365:

Update to Random_Compat 1.0.9.
This update includes fixes for Windows support & libSodium support, and removes the Throwable Polyfill due to PHP7 incompatibilities.

Fixes #28633

#70 @dd32
9 years ago

Followup for slow performance under windows: #34409 (OpenSSL times out)

#71 @dd32
9 years ago

In 35410:

Update Random_Compat to master.
This update mostly concerns OpenSSL being unusable on PHP 5.3~5.3.3.
See #28633, #34409

#72 @dd32
9 years ago

In 35600:

Update random_compat to master.
Clarifies strings, Merges [35587] upstream.
See #34409, #28633

#73 @dd32
9 years ago

I'm marking this as fixed. Please open new tickets for any further issues and reference them here.

This ticket was mentioned in Slack in #core by paragonie. View the logs.


8 years ago

Note: See TracTickets for help on using tickets.