btcutil: reject out-of-range private keys in DecodeWIF
What the developer wrote
Authored by Lrifton92
btcutil: reject out-of-range private keys in DecodeWIF
DecodeWIF did not validate that the decoded private key falls within the
valid range [1, N-1] for a secp256k1 private key. The raw 32-byte key
material was passed straight to btcec.PrivKeyFromBytes, which reduces the
scalar modulo the group order N and clamps to zero, returning no error.
As a result, a WIF encoding a private key of zero, the group order N, or
any value >= N was silently accepted. For keys >= N this is particularly
dangerous: DecodeWIF returned a private key that differs from the one
actually encoded in the WIF (e.g. a WIF for N+5 decoded to the key 5),
so an application importing such a WIF would obtain a valid-looking but
wrong key pair, with no indication that anything was off. This is also
inconsistent with hdkeychain.NewKeyFromString in the same package, which
already rejects private keys outside [1, N-1].
Validate the range using a ModNScalar (constant time): SetByteSlice
reports an overflow when the value is >= N, and IsZero covers the zero
key. Out-of-range keys now return ErrMalformedPrivateKey.
Add regression test cases (zero, N, and N+5) to TestEncodeDecodeWIF.
Signed-off-by: Lrifton92 <Lrifton92@users.noreply.github.com>
What changed, and why it matters
This commit fixes a bug in how btcd decodes Bitcoin private keys stored in the common Wallet Import Format (WIF). Previously, the software silently accepted invalid private keys, including the all-zero key and keys larger than the allowed secp256k1 group order. For keys larger than the allowed maximum, the library would quietly reduce the value modulo the group order, returning a different, valid-looking private key than the one actually encoded in the WIF. This could cause a wallet or application to import the wrong key without any warning, potentially leading to loss of funds or unexpected addresses. The fix now rejects these out-of-range keys and returns an error instead.
Upgrade to a btcd version containing this commit. Applications that import WIF strings should treat prior DecodeWIF behavior as potentially unsafe and re-validate any imported keys. Developers should audit any code paths that previously relied on DecodeWIF accepting arbitrary 32-byte values.
Security signals we found
Silent acceptance of out-of-range secp256k1 private keys
Private key scalar reduced modulo group order without error
Potential key mismatch between WIF encoding and decoded key
Inconsistent validation compared to hdkeychain.NewKeyFromString
Addition of constant-time range validation using ModNScalar
Evidence from the diff
DecodeWIF in btcutil/wif.go previously decoded the 32-byte private key payload and passed it directly to btcec.PrivKeyFromBytes, which internally reduces the scalar modulo the secp256k1 group order N and clamps zero to zero, never returning an error. As a result, WIF strings encoding private keys of 0, N, or any value >= N were accepted. For values >= N, the decoded WIF would represent a different private key than the encoded bytes (e.g., N+5 decoded to 5). The patch adds a range check using btcec.ModNScalar.SetByteSlice (which reports overflow for values >= N) and IsZero (for the zero key), returning ErrMalformedPrivateKey for any key outside [1, N-1]. Regression tests for 0, N, and N+5 are added.
Changed components
btcutil/wif.go:DecodeWIFbtcutil/wif_test.go:TestEncodeDecodeWIFInspect captured patch +39 / −0
diff --git a/btcutil/wif.go b/btcutil/wif.go
index 4ea06aa..c8aab1b 100644
--- a/btcutil/wif.go
+++ b/btcutil/wif.go
@@ -118,6 +118,21 @@ func DecodeWIF(wif string) (*WIF, error) {
netID := decoded[0]
privKeyBytes := decoded[1 : 1+btcec.PrivKeyBytesLen]
+
+ // Ensure the private key is within the valid range for a secp256k1
+ // private key, that is [1, N-1]. Without this check, a WIF encoding a
+ // key of zero or one greater than or equal to the group order N is
+ // silently accepted: btcec.PrivKeyFromBytes reduces the scalar modulo
+ // N, so DecodeWIF would otherwise return a private key that differs from
+ // the one actually encoded in the WIF (or the all-zero key) without
+ // reporting an error.
+ var keyScalar btcec.ModNScalar
+ if overflow := keyScalar.SetByteSlice(privKeyBytes); overflow ||
+ keyScalar.IsZero() {
+
+ return nil, ErrMalformedPrivateKey
+ }
+
privKey, _ := btcec.PrivKeyFromBytes(privKeyBytes)
return &WIF{privKey, compress, netID}, nil
}
diff --git a/btcutil/wif_test.go b/btcutil/wif_test.go
index 37d3e46..93b950a 100644
--- a/btcutil/wif_test.go
+++ b/btcutil/wif_test.go
@@ -122,6 +122,30 @@ func TestEncodeDecodeWIF(t *testing.T) {
wif: "5HueCGU8rMjxEXxiPuD5BDku4MkFqeZyd4dZ1jvhTVqvbTLvyTj",
err: address.ErrChecksumMismatch,
},
+ {
+ // A WIF encoding a private key of zero, which is
+ // outside the valid range [1, N-1] for a secp256k1
+ // private key.
+ name: "decodeZeroPrivKeyWif",
+ wif: "5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAbuatmU",
+ err: ErrMalformedPrivateKey,
+ },
+ {
+ // A WIF encoding a private key equal to the group order
+ // N, which is outside the valid range [1, N-1].
+ name: "decodeOrderNPrivKeyWif",
+ wif: "5Km2kuu7vtFDPpxywn4u3NLpbr5jKpTB3jsuDU2KYEqetwr388P",
+ err: ErrMalformedPrivateKey,
+ },
+ {
+ // A WIF encoding a private key of N+5, which is outside
+ // the valid range [1, N-1]. Before validation was
+ // added, this was silently reduced modulo N and decoded
+ // to a different private key (5) without any error.
+ name: "decodeAboveOrderNPrivKeyWif",
+ wif: "5Km2kuu7vtFDPpxywn4u3NLpbr5jKpTB3jsuDU2KYEqeuVhzTbv",
+ err: ErrMalformedPrivateKey,
+ },
}
for _, invalidCase := range invalidDecodeCases {
Why this scored 72/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.