software-mansion / starknet-jvm

Starknet SDK for JVM languages (Java, Kotlin, Scala)
MIT License
60 stars 15 forks source link

Possible bug in StarknetCurve #368

Closed THenry14 closed 6 months ago

THenry14 commented 7 months ago

What happened

from tg:

Tomaž Petrovič, [1 Dec 2023 at 09:13:48]: ...we've discovered a bug in the StarknetCurve when signing or verifying a signature. It happens for all hashes that are bigger than 31 bytes. Please checkout these examples.

Example of signing a hash which is 31 bytes long: Felt privateKey = Felt.fromHex("0x123"); Felt messageHash = Felt.fromHex("0x99999999999999999999999999999999999999999999999999999999999999"); StarknetCurveSignature signature = StarknetCurve.sign(privateKey, messageHash); // Result: // r: 0x5ff7c51e6fa8f98e923eca64a5f481032a82092bb57ac0b40ee27d192e1f4ac // s: 0x3bb9970580b9a20514a251d928360288883debd9ec16102e3a2cdae0ad6e727

This is correct! ✅

Example of signing a hash which is 32 bytes long: Felt privateKey = Felt.fromHex("0x123"); Felt messageHash = Felt.fromHex("0x100000000000000000000000000000000000000000000000000000000000000"); StarknetCurveSignature signature = StarknetCurve.sign(privateKey, messageHash); // Result: // r: 0x3f371a2dbe251e794e4ae278f07b11a5488f0eaa13a78db8e3598470903a421 // s: 0x33631a277b9bff0d0bd3d93a7fd62c9c609c9665c884f1456c87f8ff9c75ede

This is incorrect! ❌ The correct signature (generated with starknet.js) is: r: 0x4887f1043aa3754050fe388a163969ce0bee61223bfd68bcd6bf788ed02ba03 s: 0x746d155e164bf6a8724b9eb48a30dabfdaa50f305790c9694bf5ca0b27c7ce4

=========== indeed there seems to be a mismatch, let's see if something's changed in implementation/crypto-cpp

Stack trace

.

Steps to reproduce

SDK Version

latest

Language

Kotlin

Language

n/a

Language version

whatever we're using in repo

What operating system are you using?

Linux

Is there an existing issue for this?

DelevoXDG commented 6 months ago

TLDR: Resolved, there's no bug, the signature generated by StarknetCurve is correct, even though it's different from those generated using some other tools.

Should anyone encounter unexpected signature in the future, here's the full explanation:

The discrepancies in the hashes stem from the generation of the k-value used by ECDSA. All the implementations I've examined (cairo-lang, starknet.py, starknet-rs) base on the same algorithm. However, the implementations are a bit different.

Indeed, in case of cairo-lang and starknet-rs, the generated r, s values by default match the ones generated by starknet.js. The key point here is by default. For instance, calling cairo-lang's sign with a seed value of 32 yields a signature identical to starknet.jvm's. Seed adds additional entropy to the k-value generation as per https://datatracker.ietf.org/doc/html/rfc6979#section-3.6, but that shouldn't make k-value invalid. In starknet.py, the default seed is also set to 32, and in all implementations that allow setting it, any distinct seed produces a unique k-value, and as a result, a signature that is different from the others.

In cairo-lang and starknet-rs implementations, the default seed is null, which is equivalent to seed being a zero, or no extra entropy being added. Importantly, the seed isn't hardcoded to null or zero in any of these implementations.

While it might seem straightforward to adjust the seed in starknet-jvm to zero to generate "correct" k-values to be done with the issue, we rely on Bouncycastle to generate the k-value, and unfortunately, there appears to be no way to intervene in this process.

Additionally, all the signatures can be verified using cairo-lang's verify, with seeds set to 0, 32, and other random values. Starknet-jvm's StarknetCurve.verify() similarly raised no concerns about signatures generated with both seed 32 or seed 0.

To sum up, k-value generated by Bouncycastle is not wrong, it's just different and produces different signature, but not an incorrect one. The generated signature should still be verifiable against public key and hash:

val privateKey = Felt.fromHex("0x123")
val publicKey = StarknetCurve.getPublicKey(privateKey)
val messageHash = Felt.fromHex("0x100000000000000000000000000000000000000000000000000000000000000")
val signature = StarknetCurveSignature(
    r = Felt.fromHex("0x4887f1043aa3754050fe388a163969ce0bee61223bfd68bcd6bf788ed02ba03"),
    s = Felt.fromHex("0x746d155e164bf6a8724b9eb48a30dabfdaa50f305790c9694bf5ca0b27c7ce4")
)
val result = StarknetCurve.verify(
    publicKey = publicKey,
    hash = messageHash,
    r = signature.r,
    s = signature.s,
) // true