yomorun / hashids-java

Hashids algorithm v1.0.0 implementation in Java
http://hashids.org
MIT License
1.02k stars 156 forks source link

array index out of bound bug #31

Closed maxiwu closed 7 years ago

maxiwu commented 7 years ago

if you call the encode(number) with a random large number. then numberHashInt += (numbers[i] % (i+100)) has a chance to return a negative number, in my case -35. the next calculation use alphabet.toCharArray()[numberHashInt % alphabet.length()] could thow an ArrayIndexOutOfBoundsException.

0x3333 commented 7 years ago

Could you provide this large number?

0x3333 commented 7 years ago

This piece of code is a from the reference implementation, hashids.js.

In javascript charAt return an empty string if the index is out of bounds, in Java it throws an exception. We should range check it.

As soon as you provide the test number I'll work on it.

arcticicestudio commented 7 years ago

Maybe you want to check this block of my Hashids implementation which already handles this issue.

Anyway, isn't this issue related to #30 and should already be fixed?

0x3333 commented 7 years ago

@arcticicestudio I'll take a look later today. Thanks.

0x3333 commented 7 years ago

Well, as long as I can tell, I found a couple of problems:

  1. The numberHashInt variable is an int which in some edge cases, will overflow, generating a negative number. Here.

  2. We don't check for negative numbers in the input, we must. hashids.js

Here is a test case against master:

final long[] numbers = new long[500000];
long current = Hashids.MAX_NUMBER;
for (int i = 0; i < numbers.length; i++) {
    numbers[i] = current--;
}

final Hashids hid = new Hashids("abc");
hid.encode(numbers);

I'll check what can be done to solve these issues, but for 1, using long is probably a good call, but will generate some other problems as charAt expects an int as parameter.