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

Run $lines through array_filter() to remove empty strings that can cause unhandled exceptions #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Jul 13, 2022

In some cases, the response body pulled from the HaveIBeenPwned API can end with a blank space after the final new line characters. When pulled into the $lines array, this creates an index that consists of just a blank string.

When attempting to call list() on the result of calling explode(':', $line), an Exception is thrown.

This Exception is not caught in this package, or within Laravel NIST which uses it. The stack trace of this Exception exposes the User's password to any logs that record it.

Passing the $lines array through an array_filter() removes any blank indexes and prevents this error.

@marensas
Copy link

Currently my users cannot register or edit profile if exposed password is entered - this PR is much needed. Thanks!

@coveralls
Copy link

Pull Request Test Coverage Report for Build 176

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.0%) to 86.735%

Totals Coverage Status
Change from base Build 173: -0.0%
Covered Lines: 85
Relevant Lines: 98

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants