Check arguments passed to secp256k1 methods (#94)

* 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).

* 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)
This commit is contained in:
Fabrice Drouin 2023-12-13 13:42:14 +01:00 committed by GitHub
parent 161da89ee1
commit f242b4ffe8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 41 additions and 1 deletions

View File

@ -22,7 +22,7 @@ buildscript {
allprojects { allprojects {
group = "fr.acinq.secp256k1" group = "fr.acinq.secp256k1"
version = "0.11.0" version = "0.12.0-SNAPSHOT"
repositories { repositories {
google() google()

View File

@ -517,6 +517,7 @@ JNIEXPORT jbyteArray JNICALL Java_fr_acinq_secp256k1_Secp256k1CFunctions_secp256
if (jpubkeys == NULL) return NULL; if (jpubkeys == NULL) return NULL;
count = (*penv)->GetArrayLength(penv, jpubkeys); count = (*penv)->GetArrayLength(penv, jpubkeys);
CHECKRESULT(count < 1, "pubkey array cannot be empty")
pubkeys = calloc(count, sizeof(secp256k1_pubkey*)); pubkeys = calloc(count, sizeof(secp256k1_pubkey*));
for(i = 0; i < count; i++) { 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 (jctx == 0) return NULL;
if (jsig == NULL) return NULL; if (jsig == NULL) return NULL;
if (jmsg == NULL) return NULL; if (jmsg == NULL) return NULL;
CHECKRESULT(recid < 0 || recid > 3, "invalid recovery id");
sigSize = (*penv)->GetArrayLength(penv, jsig); sigSize = (*penv)->GetArrayLength(penv, jsig);
int sigFormat = GetSignatureFormat(sigSize); int sigFormat = GetSignatureFormat(sigSize);

View File

@ -125,6 +125,7 @@ public object Secp256k1Native : Secp256k1 {
public override fun privKeyTweakAdd(privkey: ByteArray, tweak: ByteArray): ByteArray { public override fun privKeyTweakAdd(privkey: ByteArray, tweak: ByteArray): ByteArray {
require(privkey.size == 32) require(privkey.size == 32)
require(tweak.size == 32)
memScoped { memScoped {
val added = privkey.copyOf() val added = privkey.copyOf()
val natAdd = toNat(added) val natAdd = toNat(added)
@ -136,6 +137,7 @@ public object Secp256k1Native : Secp256k1 {
public override fun privKeyTweakMul(privkey: ByteArray, tweak: ByteArray): ByteArray { public override fun privKeyTweakMul(privkey: ByteArray, tweak: ByteArray): ByteArray {
require(privkey.size == 32) require(privkey.size == 32)
require(tweak.size == 32)
memScoped { memScoped {
val multiplied = privkey.copyOf() val multiplied = privkey.copyOf()
val natMul = toNat(multiplied) val natMul = toNat(multiplied)
@ -156,6 +158,7 @@ public object Secp256k1Native : Secp256k1 {
public override fun pubKeyTweakAdd(pubkey: ByteArray, tweak: ByteArray): ByteArray { public override fun pubKeyTweakAdd(pubkey: ByteArray, tweak: ByteArray): ByteArray {
require(pubkey.size == 33 || pubkey.size == 65) require(pubkey.size == 33 || pubkey.size == 65)
require(tweak.size == 32)
memScoped { memScoped {
val nPubkey = allocPublicKey(pubkey) val nPubkey = allocPublicKey(pubkey)
val nTweak = toNat(tweak) val nTweak = toNat(tweak)
@ -166,6 +169,7 @@ public object Secp256k1Native : Secp256k1 {
public override fun pubKeyTweakMul(pubkey: ByteArray, tweak: ByteArray): ByteArray { public override fun pubKeyTweakMul(pubkey: ByteArray, tweak: ByteArray): ByteArray {
require(pubkey.size == 33 || pubkey.size == 65) require(pubkey.size == 33 || pubkey.size == 65)
require(tweak.size == 32)
memScoped { memScoped {
val nPubkey = allocPublicKey(pubkey) val nPubkey = allocPublicKey(pubkey)
val nTweak = toNat(tweak) val nTweak = toNat(tweak)
@ -175,6 +179,7 @@ public object Secp256k1Native : Secp256k1 {
} }
public override fun pubKeyCombine(pubkeys: Array<ByteArray>): ByteArray { public override fun pubKeyCombine(pubkeys: Array<ByteArray>): ByteArray {
require(pubkeys.isNotEmpty())
pubkeys.forEach { require(it.size == 33 || it.size == 65) } pubkeys.forEach { require(it.size == 33 || it.size == 65) }
memScoped { memScoped {
val nPubkeys = pubkeys.map { allocPublicKey(it).ptr } val nPubkeys = pubkeys.map { allocPublicKey(it).ptr }
@ -199,6 +204,7 @@ public object Secp256k1Native : Secp256k1 {
public override fun ecdsaRecover(sig: ByteArray, message: ByteArray, recid: Int): ByteArray { public override fun ecdsaRecover(sig: ByteArray, message: ByteArray, recid: Int): ByteArray {
require(sig.size == 64) require(sig.size == 64)
require(message.size == 32) require(message.size == 32)
require(recid in 0..3)
memScoped { memScoped {
val nSig = toNat(sig) val nSig = toNat(sig)
val rSig = alloc<secp256k1_ecdsa_recoverable_signature>() val rSig = alloc<secp256k1_ecdsa_recoverable_signature>()

View File

@ -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 @Test
fun fuzzEcdsaSignVerify() { fun fuzzEcdsaSignVerify() {
val random = Random.Default val random = Random.Default