bitwiseshiftleft / sjcl

Stanford Javascript Crypto Library
http://bitwiseshiftleft.github.com/sjcl/
Other
7.19k stars 988 forks source link

Fix bugs in bn.halveM and bn.montpowermod #441

Open Phlosioneer opened 3 months ago

Phlosioneer commented 3 months ago

montpowermod was incorrectly assuming that the base value was already modulo N. This went undetected for so long because the montMul function is VERY robust against unusually-large bases. As long as the base was within ~8 bits of the modulus, the algorithm would perform correctly. It was also partly masked by the second issue in halveM.

The halveM function did not handle halving 0 or 1 correctly. It would pop the last limb from the array, returning a bn in a weird state. Most other bn operations could recover from it, but the check that montpowermod performs on the modulus is particularly sensitive to the bug. On my machine this issue was related to the montpowermod, so I have included both bugs together in the same commit. See below.

I suspect that there is another issue in the montpowermod code, it doesn't call normalize() after RP.add(NN) and NP.add(RR), while the halveM() call assumes its input is normalized. I haven't actually encountered erroneous output, though.


On the windows build node v20.10.0, the JIT version of halveM was behaving differently from the interpreted version (i.e. while step debugging, and when the JIT was cold), for the specific input new bn(2000).powermod(800, 19), evaluating halveM on line 370:

while (RT.greaterEquals(1)) {
      RT.halveM();

The interpreted version would cause the extended Euclidean algorithm check to fail reverting to the slower (correct) powermod code.


Powermod bug was introduced in commit 2f591b4

HalveM bug was introduced in commit c08108f

Closes #419

Related to #172