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

BigInt::random_integer hangs #3590

Closed
guidovranken opened this issue Jun 21, 2023 · 3 comments · Fixed by #3594
Closed

BigInt::random_integer hangs #3590

guidovranken opened this issue Jun 21, 2023 · 3 comments · Fixed by #3594

Comments

@guidovranken
Copy link

#include <botan/bigint.h>
#include <botan/system_rng.h>

int main(void)
{
    using namespace Botan;
    System_RNG rng;

    BigInt A("0x1fec2433a4401374b0220cf2f92f7ac3a5a84610ff4896118525dfc6a556dfbf9ce2698c20cfc780");
    BigInt B("0x1fec2433a4401374b0220cf2f92f8735d4b73fe909f5fa3658d30b423d4630b4dce2698c20cfc780");

    BigInt::random_integer(rng, A, B);

    return 0;
}
@guidovranken
Copy link
Author

guidovranken commented Jun 21, 2023

This is random_integer:

   const size_t bits = max.bits();

   do {
      r.randomize(rng, bits, false);
   } while(r < min || r >= max);

I guess it will finish eventually, but it might be better to do generate an int in the range 0..max-min and then add min.

@randombit
Copy link
Owner

Confirmed, thanks. We'll have to be careful about changing the logic as a number of tests rely on the current behavior, but obviously taking forever on certain inputs is no good. I'll take a look.

randombit added a commit that referenced this issue Jun 23, 2023
We maintain a distinct codepath for the case where min is 0 or 1,
since otherwise certain tests which expect stability of this function
will break.

If min is > 1, we generate a number in the range `min + [0,max-min)`,
which avoids the slow case.

Fixes #3590
@randombit
Copy link
Owner

@guidovranken Fixed now

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 a pull request may close this issue.

2 participants