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.

Sign up for GitHub

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

Jump to bottom

investigate potential regression in 1.2.0 #23

Closed
paolostivanin opened this issue Aug 19, 2018 · 4 comments · Fixed by #24
Closed

investigate potential regression in 1.2.0 #23

paolostivanin opened this issue Aug 19, 2018 · 4 comments · Fixed by #24
Assignees
@paolostivanin
Labels
Status: Done Type: Bug

Comments

@paolostivanin
Copy link
Owner

#22 (comment)
@paolostivanin paolostivanin self-assigned this Aug 19, 2018
@paolostivanin
Copy link
Owner Author

@magiruuvelvet please let's continue the discussion here, thx :)

@magiruuvelvet
Copy link
Contributor

magiruuvelvet commented Aug 20, 2018 •
edited
Loading

DIGITS_POWER[digits_length]

There is an array out of bound when the digit size is greater than 8. I just increased the array with more values.

I'm on Linux btw and compiled with clang. The behavior is undefined, but should normally result in a segfault I guess.

@paolostivanin
Copy link
Owner Author

paolostivanin commented Aug 21, 2018 •
edited
Loading

Yep, you're right! I was able to reproduce the issue with >= 15 digits. Increasing the array size is not enough though, there are some other fixes that are needed :)

paolostivanin added a commit that referenced this issue Aug 21, 2018
@paolostivanin
Set max digits to 10
a481b1c 
- fix #23
- support for bigger tokens will come in future, if ever needed (10 digits should be more than enough for the time being)
- update readme
- add test
@paolostivanin paolostivanin mentioned this issue Aug 21, 2018
Merged
@paolostivanin
Copy link
Owner Author

paolostivanin commented Aug 21, 2018 •
edited
Loading

Supporting tokens with long digits (> 10) would require a bigger refactoring and I don't really have time for that. I want to focus on OTPClient v1.3.0 rather than supporting a non existing use case :)
I looked online and no one is using more than 8 digits, so for the time being the max supported digits will be 10

paolostivanin added a commit that referenced this issue Aug 21, 2018
@paolostivanin
Set max digits to 10
57d42e9 
- fix #23
- support for bigger tokens will come in future, if ever needed (10 digits should be more than enough for the time being)
- update readme
- add test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@paolostivanin paolostivanin

Labels
Status: Done Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set max digits to 10 paolostivanin/libcotp
2 participants
@paolostivanin @magiruuvelvet