Skip to content

Client accepts CSN which are incremented by more than one. #136

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

Open
rustonaut opened this issue Sep 14, 2021 · 1 comment
Open

Client accepts CSN which are incremented by more than one. #136

rustonaut opened this issue Sep 14, 2021 · 1 comment
Labels

Comments

@rustonaut
Copy link

Following code checks the CSN:

final long previous = peer.getCsnPair().getTheirs();
final long current = nonce.getCombinedSequence();
if (current < previous) {
throw new ValidationError(peer.getName() + " CSN is lower than last time");
} else if (current == previous) {
throw new ValidationError(peer.getName() + " CSN hasn't been incremented");
} else {
peer.getCsnPair().setTheirs(current);
}

accepting any CSN which is larger then the previous CSN.

But the spec states it must have been incremented by 1:

If the message is received by a client or received by and intended for a server (the destination address is 0x00), the peer does the following checks:
[...]

  • In case that the peer does make use of the combined sequence number, it MUST check that the combined sequence number of the source peer has been increased by 1 and has not reset to 0. Implementations that use the combined sequence number SHALL ignore the following three checks.
@lgrahl lgrahl added the bug label Sep 15, 2021
@lgrahl
Copy link
Member

lgrahl commented Sep 15, 2021

It should be noted that although this being a spec violation it's not security relevant since the CSN is only allowed to monotonically increase.

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

2 participants