Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security::hash() should throw an exception if algo not found #11253

Closed
2 tasks done
inoas opened this issue Sep 28, 2017 · 4 comments
Closed
2 tasks done

Security::hash() should throw an exception if algo not found #11253

inoas opened this issue Sep 28, 2017 · 4 comments

Comments

@inoas
Copy link
Contributor

inoas commented Sep 28, 2017

This is a (multiple allowed):

  • bug

  • enhancement

  • CakePHP Version: 3.4.2

What you did

$hash = Security::hash('foobar', 'sha512a');

sha512a doesn't exist, only sha512 does (on my system).

What happened

Warning

What you expected to happen

Exception

Reason: This should break your application not just throw a warning for better security.
Result was that $hash became false. Labeled as bug just cause I consider this bad practice for security.

I could probably create a PR if I can make the test cases catch the exception.
This should probably happen for 4.x as it is non-BC?

@dereuromark
Copy link
Member

In 3.6 the exception sure makes sense. For 3.5 we could think about triggering a warning instead.

@markstory
Copy link
Member

Adding exceptions is acceptable as far as backwards compatibility is concerned. Would you be able to put a pull request together @inoas ?

@inoas
Copy link
Contributor Author

inoas commented Sep 28, 2017

I suppose I can - I'll take a look this weekend / next week.

@inoas
Copy link
Contributor Author

inoas commented Oct 6, 2017

Closing as PR is open.

@inoas inoas closed this as completed Oct 6, 2017
markstory added a commit that referenced this issue Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants