Skip to content

Commit 183e793

Browse files
committed
Bugfix: invalid resource type passed in CompactSignature
In the secp256k1 CompactSignature implementation we pass the recoverable signature instead of the signature.. This is a bug, although it does make us wonder what the goal of this design decision was (to extend Signature)
1 parent 1aab3ee commit 183e793

File tree

3 files changed

+14
-11
lines changed

3 files changed

+14
-11
lines changed

src/Crypto/EcAdapter/Impl/Secp256k1/Signature/CompactSignature.php

+10-7
Original file line numberDiff line numberDiff line change
@@ -33,31 +33,34 @@ class CompactSignature extends Signature implements CompactSignatureInterface
3333

3434
/**
3535
* @param EcAdapter $ecAdapter
36-
* @param resource $secp256k1_ecdsa_signature_t
36+
* @param resource $secp256k1_recoverable_signature_t
3737
* @param int $recid
3838
* @param bool $compressed
3939
*/
40-
public function __construct(EcAdapter $ecAdapter, $secp256k1_ecdsa_signature_t, int $recid, bool $compressed)
40+
public function __construct(EcAdapter $ecAdapter, $secp256k1_recoverable_signature_t, int $recid, bool $compressed)
4141
{
42-
if (!is_resource($secp256k1_ecdsa_signature_t) || SECP256K1_TYPE_RECOVERABLE_SIG !== get_resource_type($secp256k1_ecdsa_signature_t)) {
43-
throw new \RuntimeException('CompactSignature: must pass recoverable signature resource');
42+
if (!is_resource($secp256k1_recoverable_signature_t) || SECP256K1_TYPE_RECOVERABLE_SIG !== get_resource_type($secp256k1_recoverable_signature_t)) {
43+
throw new \RuntimeException('CompactSignature: must pass recoverable signature resource, not ' . $secp256k1_recoverable_signature_t);
4444
}
4545

4646
$ser = '';
4747
$recidout = 0;
48-
secp256k1_ecdsa_recoverable_signature_serialize_compact($ecAdapter->getContext(), $ser, $recidout, $secp256k1_ecdsa_signature_t);
48+
secp256k1_ecdsa_recoverable_signature_serialize_compact($ecAdapter->getContext(), $ser, $recidout, $secp256k1_recoverable_signature_t);
4949
list ($r, $s) = array_map(
5050
function ($val) {
5151
return (new Buffer($val))->getGmp();
5252
},
5353
str_split($ser, 32)
5454
);
5555

56-
$this->resource = $secp256k1_ecdsa_signature_t;
56+
$this->resource = $secp256k1_recoverable_signature_t;
5757
$this->recid = $recid;
5858
$this->compressed = $compressed;
5959
$this->ecAdapter = $ecAdapter;
60-
parent::__construct($ecAdapter, $r, $s, $secp256k1_ecdsa_signature_t);
60+
61+
$sig_t = null;
62+
secp256k1_ecdsa_recoverable_signature_convert($this->ecAdapter->getContext(), $sig_t, $this->resource);
63+
parent::__construct($ecAdapter, $r, $s, $sig_t);
6164
}
6265

6366
/**

tests/Crypto/EcAdapter/Impl/Secp256k1/Signature/CompactSignatureTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function testNotResource()
2020
if (!function_exists('secp256k1_context_create')) {
2121
$this->markTestSkipped("secp256k1 not installed");
2222
}
23-
$this->expectException(\InvalidArgumentException::class);
23+
$this->expectException(\RuntimeException::class);
2424
$this->expectExceptionMessage('CompactSignature: must pass recoverable signature resource');
2525
$this->callConstructor("");
2626
}
@@ -29,7 +29,7 @@ public function testWrongResourceType()
2929
if (!function_exists('secp256k1_context_create')) {
3030
$this->markTestSkipped("secp256k1 not installed");
3131
}
32-
$this->expectException(\InvalidArgumentException::class);
32+
$this->expectException(\RuntimeException::class);
3333
$this->expectExceptionMessage('CompactSignature: must pass recoverable signature resource');
3434
$this->callConstructor(gmp_init(1));
3535
}

tests/Crypto/EcAdapter/Impl/Secp256k1/Signature/SignatureTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function testNotResource()
2020
$this->markTestSkipped("secp256k1 not installed");
2121
}
2222
$this->expectException(\InvalidArgumentException::class);
23-
$this->expectExceptionMessage('CompactSignature: must pass recoverable signature resource');
23+
$this->expectExceptionMessage('Secp256k1\Signature\Signature expects ' . SECP256K1_TYPE_SIG . ' resource');
2424
$this->callConstructor("");
2525
}
2626
public function testWrongResourceType()
@@ -29,7 +29,7 @@ public function testWrongResourceType()
2929
$this->markTestSkipped("secp256k1 not installed");
3030
}
3131
$this->expectException(\InvalidArgumentException::class);
32-
$this->expectExceptionMessage('CompactSignature: must pass recoverable signature resource');
32+
$this->expectExceptionMessage('Secp256k1\Signature\Signature expects ' . SECP256K1_TYPE_SIG . ' resource');
3333
$this->callConstructor(gmp_init(1));
3434
}
3535
}

0 commit comments

Comments
 (0)