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

array index out of bound bug #31

Closed
maxiwu opened this issue Dec 9, 2016 · 5 comments
Closed

array index out of bound bug #31

maxiwu opened this issue Dec 9, 2016 · 5 comments
Assignees
Labels

Comments

@maxiwu
Copy link

maxiwu commented Dec 9, 2016

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
Copy link
Collaborator

0x3333 commented Dec 9, 2016

Could you provide this large number?

@0x3333 0x3333 self-assigned this Dec 9, 2016
@0x3333
Copy link
Collaborator

0x3333 commented Dec 9, 2016

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
Copy link

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
Copy link
Collaborator

0x3333 commented Dec 12, 2016

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

@0x3333
Copy link
Collaborator

0x3333 commented Dec 13, 2016

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.

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

No branches or pull requests

4 participants