Skip to content

Wrong giant.sign from gigimport #9

@JayFoxRox

Description

@JayFoxRox

xbedump/giants.c

Lines 320 to 335 in d0d6508

bitlen(
giant n
)
{
int b = 16, c = 1<<15, w;
if (isZero(n))
return(0);
w = n->n[abs(n->sign) - 1];
while ((w&c) == 0)
{
b--;
c >>= 1;
}
return (16*(abs(n->sign)-1) + b);
}

Does not work if w is zero at start of loop (in trivial values like 0000 0001).

w&c will never be true, so the loop never exits.

At first I thought the correct condition would probably be c && (w&c) so it counts.


However, the function is weird to begin with:

0000 0000 = len 0 (n is zero: special case)
0001 0000 = len 17 ('w' is 1 bit, sign is 2 → 1 * 16 bit)
0000 0001 = len 16 (w is 0 bit, but sign is 2 → 1 * 16 bit); but why isn't this 1?

I wonder if this has to do with the removal in #5 (see #4 for a poor description of this).

From what I can tell, it didn't make sense to have numbers with zero-words in the top words (you'd just decrement sign instead).


So I also looked for where the code came from, and found https://github.com/rudimeier/mprime/blob/master/gwnum/giants.c which is a 32 bit port. It also has an assert for g->n[abs(g->sign)-1] != 0 which warns about w being zero.

So we should:

  • Fix our gigimport and add the license text.
  • Consider an upgrade to the 32 bit (or even 64 bit) lib and add the license text.
  • Consider switching to an entirely different RSA implementation.

The same bugs also plague @Cxbx-Reloaded which uses the same giants.c in its EmuRSA.cpp (which is also a copyright violation: it doesn't contain the copyright strings, which are mandatory according to the license)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions