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

Nonce increment incorrect when multiple bytes need changing #9

Closed
1 task done
causalnet opened this issue Jan 15, 2023 · 4 comments · Fixed by #10
Closed
1 task done

Nonce increment incorrect when multiple bytes need changing #9

causalnet opened this issue Jan 15, 2023 · 4 comments · Fixed by #10
Labels
bug Something isn't working confirmed This is a bug

Comments

@causalnet
Copy link
Contributor

Please agree to the following

Summary

Every so often, calls to keepassxc get stuck, which seems to be caused by nonce increment code not matching keepassxc's incremented nonces when multiple bytes need to be incremented

What software is involved?

KeePassXC 2.7.4
Reproduced bug on both Windows and Linux OSes.

Steps to Reproduce

Keep making KeepassXC calls, such as Connection.getLogins() although it can happen with any call that requires a response that needs incremented nonce matching. Most will work, but about 1 in 256 will get stuck and block forever. Debugging it, this seemed to be due to the nonce of KeepassXC's response not matching what Connection.incrementNonce() has calculated it should be.

This happens when the last nonce byte rolls over from 255 to 0. Proxy access's increment code only increments the last byte, but KeepassXC's will increment the next byte (and so on) in this case.

Expected Behavior

Nonce should match and call should work every time.

Actual Behavior

Occasionally calls get stuck and block forever.

Reproducibility

Intermittent

Relevant Log Output

No response

Anything else?

No response

@causalnet
Copy link
Contributor Author

I wrote a repeating test case in my fork that you might be able to use to reproduce this behaviour. Since the generated nonce is random, I guess there's a 1/256 chance of getting the last byte of the nonce to be 255 which requires the next byte to be incremented.

I also made an additional unit test in another branch for testing just the nonce increment code, but haven't added this to the PR. This is the commit. I can add it if you want.

@purejava
Copy link
Owner

I also made an additional unit test in another branch for testing just the nonce increment code, but haven't added this to the PR. This is the commit. I can add it if you want.

Thanks, that won't be necessary. I can find the unit test there.

Hopefully I'll find the time to look into this issue within this week, as unfortunately I am quite busy right now.

@purejava
Copy link
Owner

Thank you for your thorough analysis, for opening this issue, writing the unit test and developing a fix.

I am preparing a new release containing your fix now.

@purejava purejava added the confirmed This is a bug label Jan 17, 2023
@causalnet
Copy link
Contributor Author

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants