Skip to content

Conversation

@czurnieden
Copy link
Contributor

Bugfix regarding issue #486.

I cannot imagine how that issue might be exploited but because I cannot imagine it doesn't mean there is no way at all.

The extra checks are cheap and the function itself should even be a little bit faster together with less need for stack and heap with the other changes.

I did not pick any reviewer explicitly because of the current situation and its accompanying mess but I would appreciate it highly if somebody would take a look. Thank's!

Copy link
Member

@sjaeckel sjaeckel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@friedrichsenm can you please verify

Copy link
Collaborator

@friedrichsenm friedrichsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one question about a kronecker symbol check in mp_sqrtmod_prime.c. When checking (n | prime) it might make sense to check instead that legendre != 1 instead of legendre == -1 to handle the case when it is 0.

@czurnieden
Copy link
Contributor Author

When checking (n | prime) it might make sense to check instead that legendre != 1 instead of legendre == -1 to handle the case when it is 0.

Yepp, missed that. Good find, thanks!

@sjaeckel sjaeckel merged commit c18817c into libtom:develop Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants