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

The count is counting all hits, not the matched password found #1

Closed
2 tasks done
DragonBe opened this issue Jun 13, 2018 · 1 comment
Closed
2 tasks done

The count is counting all hits, not the matched password found #1

DragonBe opened this issue Jun 13, 2018 · 1 comment
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@DragonBe
Copy link
Owner

  • I was not able to find an open or closed issue matching what I'm seeing.
  • This is not a question.

When running count($hibp) it returns a total of all returned hash counts, not the matched count.

Code to reproduce the issue

<?php

require_once __DIR__ . '/vendor/autoload.php';

$hibp = \Dragonbe\Hibp\HibpFactory::create();
$found = $hibp->isPwnedPassword('password');
$count = count($hibp);
echo 'Password "password": ' . (
    $found
    ? 'Pwned (found ' . $count . ' times)'
    : 'Not used in a breach yet'
    ) . PHP_EOL;

Gives:

Password "password": Pwned (found 3311463 times)

Expected results

Going to HIBP it returns for password "password":

This password has been seen 3,303,003 times before

Actual results

There's an issue on lines 96 - 98 in src/Hibp.php:

   list($hash, $count) = explode(':', $value);
   $totalCount += $count;
   return (0 === strcmp($hash, substr($password, 5)));

It should be changed into:

   list($hash, $count) = explode(':', $value);
   if (0 === strcmp($hash, substr($password, 5))) {
       $totalCount = $count;
       return true;
   }
   return false;
@DragonBe DragonBe added bug Something isn't working good first issue Good for newcomers labels Jun 13, 2018
@DragonBe DragonBe self-assigned this Jun 14, 2018
DragonBe added a commit that referenced this issue Jun 14, 2018
We need to return the amount of times a specific password was found in a breach, not a total of hits of all returned hashes. See issue #1 for details.
DragonBe added a commit that referenced this issue Jun 14, 2018
Solving issue #1 on a wrong count of found passwords in HIBP
DragonBe added a commit that referenced this issue Jun 14, 2018
Fix for issue #1: making sure the count of hits on password are for the password itself and not for all hashes returned by HIBP.
@DragonBe
Copy link
Owner Author

Solved in commit e871df6 and released as v0.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant