Fix curvePoint.Equal() mutating its operands#626
Conversation
Equal() was calling big.Int.Mod in place on both operands' coordinates, which violated the read-only contract of an equality test. Additionally, Set(), Clone(), and Base() copied big.Int pointers rather than values, meaning mutation in Equal could corrupt aliased points — including the curve's global generator coordinates. Fix by using temporary big.Ints in Equal and deep-copying in Set, Clone, and Base. Fixes dedis#625 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer. |
|
@thehoul and @AnomalRoil, are you able to provide any estimate on how long you will require for PR review? I'm just trying to estimate how long we'll need to keep the work-around in our codebase. |
1 similar comment
|
@thehoul and @AnomalRoil, are you able to provide any estimate on how long you will require for PR review? I'm just trying to estimate how long we'll need to keep the work-around in our codebase. |
|
|
@eljobe I can give it a review on Monday. If you want to speed things up on my side you could run your test against the v3 codebase just to confirm whether this is a regression in v4 or has been there for a while. |
Yes. It also fails in v3.1.1. Reproduction steps: Would it be helpful for me to push that branch to my fork? |



Summary
Equal()was callingbig.Int.Modin place on both the receiver's and argument'sx/ycoordinates, mutating them as a side effect of what should be a read-only comparison. Fixed by using temporarybig.Intvalues.Set()andClone()copiedbig.Intpointers rather than values, so any mutation (e.g. viaEqual) on one point would silently corrupt aliased points. Fixed by deep-copying withnew(big.Int).Set(...).Base()assigned the curve's globalGx/Gypointers directly, meaning mutations could corrupt the canonical generator. Fixed by deep-copying.Fixes #625
Test plan
TestEqualDoesNotMutate— verifiesEqualdoesn't normalize the receiverTestEqualDoesNotMutateArgument— verifiesEqualdoesn't normalize the argumentTestSetDeepCopies— verifiesSetdoesn't aliasbig.IntpointersTestCloneDeepCopies— verifiesClonedoesn't aliasbig.IntpointersTestBaseDeepCopies— verifiesBasedoesn't alias curve generator pointersgroup/p256tests pass (TestQR512,TestP256,TestSetBytesBE,TestVectors)🤖 Generated with Claude Code