From af9615642e6ad57f0ac4ccd918c03f5e620e205d Mon Sep 17 00:00:00 2001 From: sstone Date: Wed, 13 Dec 2023 10:12:28 +0100 Subject: [PATCH 1/3] Set version to 0.12.0-SNAPSHOT --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 537b2d9..75f594e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -22,7 +22,7 @@ buildscript { allprojects { group = "fr.acinq.secp256k1" - version = "0.11.0" + version = "0.12.0-SNAPSHOT" repositories { google() From f8f1356ef840f3cb956b6a15019091e70ef90a3b Mon Sep 17 00:00:00 2001 From: sstone Date: Wed, 13 Dec 2023 11:25:22 +0100 Subject: [PATCH 2/3] Check arguments passed to secp256k1 methods Illegal arguments will trigger an internal callback that prints to stderr and calls abort. We already check arguments in our JNI and kotlin native code but had missed 2 checks (recid in ecdsaRecover, empty arrays in pubkeyCombine). --- .../fr_acinq_secp256k1_Secp256k1CFunctions.c | 2 ++ .../fr/acinq/secp256k1/Secp256k1Native.kt | 2 ++ .../fr/acinq/secp256k1/Secp256k1Test.kt | 32 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/jni/c/src/fr_acinq_secp256k1_Secp256k1CFunctions.c b/jni/c/src/fr_acinq_secp256k1_Secp256k1CFunctions.c index 162a5c2..4bc7b0a 100644 --- a/jni/c/src/fr_acinq_secp256k1_Secp256k1CFunctions.c +++ b/jni/c/src/fr_acinq_secp256k1_Secp256k1CFunctions.c @@ -517,6 +517,7 @@ JNIEXPORT jbyteArray JNICALL Java_fr_acinq_secp256k1_Secp256k1CFunctions_secp256 if (jpubkeys == NULL) return NULL; count = (*penv)->GetArrayLength(penv, jpubkeys); + CHECKRESULT(count < 1, "pubkey array cannot be empty") pubkeys = calloc(count, sizeof(secp256k1_pubkey*)); for(i = 0; i < count; i++) { @@ -600,6 +601,7 @@ JNIEXPORT jbyteArray JNICALL Java_fr_acinq_secp256k1_Secp256k1CFunctions_secp256 if (jctx == 0) return NULL; if (jsig == NULL) return NULL; if (jmsg == NULL) return NULL; + CHECKRESULT(recid < 0 || recid > 3, "invalid recovery id"); sigSize = (*penv)->GetArrayLength(penv, jsig); int sigFormat = GetSignatureFormat(sigSize); diff --git a/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt b/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt index 7ffe516..2c47201 100644 --- a/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt +++ b/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt @@ -175,6 +175,7 @@ public object Secp256k1Native : Secp256k1 { } public override fun pubKeyCombine(pubkeys: Array): ByteArray { + require(pubkeys.isNotEmpty()) pubkeys.forEach { require(it.size == 33 || it.size == 65) } memScoped { val nPubkeys = pubkeys.map { allocPublicKey(it).ptr } @@ -199,6 +200,7 @@ public object Secp256k1Native : Secp256k1 { public override fun ecdsaRecover(sig: ByteArray, message: ByteArray, recid: Int): ByteArray { require(sig.size == 64) require(message.size == 32) + require(recid in 0..3) memScoped { val nSig = toNat(sig) val rSig = alloc() diff --git a/tests/src/commonTest/kotlin/fr/acinq/secp256k1/Secp256k1Test.kt b/tests/src/commonTest/kotlin/fr/acinq/secp256k1/Secp256k1Test.kt index c36ef50..cf07ae1 100644 --- a/tests/src/commonTest/kotlin/fr/acinq/secp256k1/Secp256k1Test.kt +++ b/tests/src/commonTest/kotlin/fr/acinq/secp256k1/Secp256k1Test.kt @@ -352,6 +352,38 @@ class Secp256k1Test { } } + @Test + fun testInvalidArguments() { + assertFails { + Secp256k1.pubkeyCreate(ByteArray(32)) + } + assertFails { + Secp256k1.pubkeyCreate(Hex.decode("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")) + } + assertFails { + Secp256k1.pubkeyParse(ByteArray(33)) + } + assertFails { + Secp256k1.pubkeyParse(Hex.decode("03ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")) + } + assertFails { + Secp256k1.pubKeyCombine(arrayOf()) + } + assertFails { + Secp256k1.pubKeyCombine(arrayOf(ByteArray(0))) + } + assertFails { + Secp256k1.signSchnorr(ByteArray(0), Hex.decode("0101010101010101010101010101010101010101010101010101010101010101"), null) + } + assertFails { + Secp256k1.ecdsaRecover( + Hex.decode("01010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101"), + Hex.decode("0202020202020202020202020202020202020202020202020202020202020202"), + -1 + ) + } + } + @Test fun fuzzEcdsaSignVerify() { val random = Random.Default From 8010f4fd441c64c2cf97779c209da5a10762b048 Mon Sep 17 00:00:00 2001 From: sstone Date: Wed, 13 Dec 2023 13:20:17 +0100 Subject: [PATCH 3/3] Implement the same "tweak" checks in the native code and JNI code The native code was missing checks on the "tweak" size (which must be 32 bytes) --- src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt b/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt index 2c47201..2f0db83 100644 --- a/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt +++ b/src/nativeMain/kotlin/fr/acinq/secp256k1/Secp256k1Native.kt @@ -125,6 +125,7 @@ public object Secp256k1Native : Secp256k1 { public override fun privKeyTweakAdd(privkey: ByteArray, tweak: ByteArray): ByteArray { require(privkey.size == 32) + require(tweak.size == 32) memScoped { val added = privkey.copyOf() val natAdd = toNat(added) @@ -136,6 +137,7 @@ public object Secp256k1Native : Secp256k1 { public override fun privKeyTweakMul(privkey: ByteArray, tweak: ByteArray): ByteArray { require(privkey.size == 32) + require(tweak.size == 32) memScoped { val multiplied = privkey.copyOf() val natMul = toNat(multiplied) @@ -156,6 +158,7 @@ public object Secp256k1Native : Secp256k1 { public override fun pubKeyTweakAdd(pubkey: ByteArray, tweak: ByteArray): ByteArray { require(pubkey.size == 33 || pubkey.size == 65) + require(tweak.size == 32) memScoped { val nPubkey = allocPublicKey(pubkey) val nTweak = toNat(tweak) @@ -166,6 +169,7 @@ public object Secp256k1Native : Secp256k1 { public override fun pubKeyTweakMul(pubkey: ByteArray, tweak: ByteArray): ByteArray { require(pubkey.size == 33 || pubkey.size == 65) + require(tweak.size == 32) memScoped { val nPubkey = allocPublicKey(pubkey) val nTweak = toNat(tweak)