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

Fix: Truncate key-length when generating identicons #341

Merged
merged 1 commit into from Aug 17, 2017

Conversation

Projects
None yet
2 participants
@macedigital
Contributor

macedigital commented Aug 6, 2017

Hex-digits with character count above 17 cannot be safely converted to an Integer, see MAX_SAFE_INTEGER.

Therefore, when long keys (e.g. 32 characters) are passed into generateIdenticon(), and the modulus of 2^18 is performed, the result is 0 all the time. This means, the identicon will render as an empty svg image.

Here is a proof-of-concept (run in any modern browser):

const key = '841b625dcf75413ff3ed5137a81ff1c3';
const int = parseInt(key, 16);
const hash = int % Math.pow(2, 18);
// throws, due to floating point conversion / integer overflow
console.assert(258499 === hash, "Modulus for 'hash' should be != 0");

const int2 = parseInt(key.substr(-16), 16);
const hash2 = int2 % Math.pow(2, 18);
// works as expected
console.assert(258048 === hash2, "Modulus 'hash2' should be != 0");

Truncating the passed in argument to a maximum of 16 characters solves the issue.

As a sidenote, the same code in Python will work correctly:

key = '841b625dcf75413ff3ed5137a81ff1c3'
int = int(key, 16)
hash = int % pow(2, 18)
assert 258499 == hash
Fix: Truncate key-length when generating identicons
Hex-digits with character count above 17 cannot be safely converted to an Integer, see [MAX_SAFE_INTEGER](https://medium.com/the-node-js-collection/javascripts-number-type-8d59199db1b6#53cd).

Therefore, when long keys (e.g. 32 characters) are passed into `generateIdenticon()`, and the modulus of 2^18 is performed, the result is 0 all the time. This means, the identicon will render as an empty svg image.

Here is a proof-of-concept (run in any modern browser):

```js
const key = '841b625dcf75413ff3ed5137a81ff1c3';
const int = parseInt(key, 16);
const hash = int % Math.pow(2, 18);
// throws, due to floating point conversion / integer overflow
console.assert(258499 === hash, "Modulus for 'hash' should be != 0");

const int2 = parseInt(key.substr(-16), 16);
const hash2 = int2 % Math.pow(2, 18);
// works as expected
console.assert(258048 === hash2, "Modulus 'hash2' should be != 0");
```

Truncating the passed in argument to a maximum of 16 characters solves the issue.

As a sidenote, the same code in Python will work correctly:

```python
key = '841b625dcf75413ff3ed5137a81ff1c3'
int = int(key, 16)
hash = int % pow(2, 18)
assert 258499 == hash
```

@posativ posativ merged commit 650c6cf into posativ:master Aug 17, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@posativ

This comment has been minimized.

Owner

posativ commented Aug 17, 2017

Thanks for the fix and explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment