? Error

User tests: Successful: Unsuccessful:

avatar dunglas
dunglas
2 Sep 2014

Use the hash_equals function (introduced in PHP 5.6) for timing attack safe string comparison when available.
Add in the DocBlock that length will leak (see php/php-src#792).

avatar dunglas dunglas - open - 2 Sep 2014
avatar jissues-bot jissues-bot - change - 2 Sep 2014
Labels Added: ?
avatar wilsonge
wilsonge - comment - 2 Sep 2014

#3832

Similar issue, alternative fix, we received a while back (less good but full PHP version support)

avatar dunglas
dunglas - comment - 2 Sep 2014

@wilsonge I don't know if this better to use HMAC or hash_equals for passwords but, if this method is part of the Joomla API both patch can be merged. It will benefit at least to plugins using it.

avatar sarciszewski
sarciszewski - comment - 7 Oct 2014

:+1: for using native hash_equals() if it exists.

avatar brianteeman brianteeman - change - 17 Oct 2014
Category Libraries
avatar RCheesley
RCheesley - comment - 18 Oct 2014

Can you please provide test instructions?

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4206.

avatar brianteeman
brianteeman - comment - 2 Jan 2015

As this requires php 5.6 which is higher than our minimum requirement how can this be merged?

Setting to Needs Review so the CMS maintainers can make a decision


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4206.

avatar brianteeman brianteeman - change - 2 Jan 2015
Status Pending Needs Review
avatar sarciszewski
sarciszewski - comment - 2 Jan 2015

As this requires php 5.6 which is higher than our minimum requirement how can this be merged?

Have you even read what the pull request does? It's wrapped inside of a if (function_exists('hash_equals')) block...

avatar Hackwar
Hackwar - comment - 3 Feb 2015

The PR looks good, but please update the PR to latest staging so that Travis is run and can give us a proper result.

avatar mbabker
mbabker - comment - 21 Aug 2015

I don't see an issue with this. We've done similar things in the past where we use native PHP functions conditionally and fallback to something else if it isn't available. As requested previously though, can this PR be sync'd with the staging branch so it can be properly tested by both users and the CI suite? Also, there is one codestyle issue to fix; the opening bracket for the if statement should be on a new line.

avatar zero-24
zero-24 - comment - 23 Aug 2015

This PR: #7754 fixes the codestyle issue and replaces this here. Thanks @dunglas !

avatar zero-24 zero-24 - change - 23 Aug 2015
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2015-08-23 17:38:10
Closed_By zero-24
avatar zero-24 zero-24 - close - 23 Aug 2015

Add a Comment

Login with GitHub to post a comment