Issue 264 in bitcoinj: ECKey constructor allows bogus key creation

[bitc...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Enhancement Priority-Medium

New issue 264 by n.e.baughman: ECKey constructor allows bogus key creation
http://code.google.com/p/bitcoinj/issues/detail?id=264

bitcoinj-0.6

The ECKey(byte[] privKeyBytes, byte[] pubKey) constructor is misleading.
The API docs explain that "The public key will be automatically derived
from the private key." However, providing a zero-length pubKey argument
results in an ECKey with a bogus (but valid-looking) public key.

new ECKey(privKeyBytes, new byte[0]).toAddress() yields an address
of "1HT7xU2Ngenf7D4yocz2SAcnNLW7rK8d4E" which is a known bogus address.
Google search yields:

https://bitcointalk.org/index.php?topic=8728.25;wap2
"... (an address derived from pubkey of length 0 which cannot exist)."

Minimally, the API docs should be made clear that the constructor blindly
accepts the values given.

Notice that the documented behavior is impossible without the
NetworkParameters (needed to derive the public key), so a privKeyBytes-only
constructor is not possible. Consider a static method, such as:
ECKey.create(byte[] privKeyBytes, NetworkParameters params)

[bitc...@googlecode.com]

Comment #1 on issue 264 by he...@google.com: ECKey constructor allows bogus
key creation
http://code.google.com/p/bitcoinj/issues/detail?id=264

The API docs can be clearer, but is "empty array == no input" an idiom you
have observed anywhere? I don't think it's used anywhere in bitcoinj nor
any other Java API I am aware of.

We can make the ECKey API better and add more sanity checks, but it's a
really bad idea to pass in garbage data to function parameters especially
when doing crypto.

You don't need a NetworkParameters to derive a pubkey. It is assumed that
all networks use secp256k1. Network parameters are only relevant for
addresses, not keys.

[bitc...@googlecode.com]

Comment #2 on issue 264 by n.e.baughman: ECKey constructor allows bogus key
creation
http://code.google.com/p/bitcoinj/issues/detail?id=264

Thank you for your corrections. I'm looking at the ECKey source now (should
have before). I have no idea where I came up with passing an empty byte[];
bad form to be sure. In retrospect, null would be the only rational choice
(which looks like the expected case from the constructor source).

Sorry for the key/address confusion, too. I'm a bit overworked, I guess.
I'll strive to find better issues in the future.

If anything, a small documentation clarification is probably all this boils
down to.

[bitc...@googlecode.com]

Updates:
Status: Fixed

Comment #3 on issue 264 by he...@google.com: ECKey constructor allows bogus
key creation
http://code.google.com/p/bitcoinj/issues/detail?id=264

I fixed the docs (will push to master later). There's already a TODO in the
code to redesign the ECKey API.

[bitc...@googlecode.com]

Comment #4 on issue 264 by kexi...@diyism.com: ECKey constructor allows

bogus key creation
http://code.google.com/p/bitcoinj/issues/detail?id=264

in php:

<?php
function pubkey_to_address($pubkey)
{if (strlen($pubkey)!==65)
{return false;
}
$pre_pubkey='00';
$hash=hash('ripemd160', hash('sha256', $pubkey, 1));
$hash=$pre_pubkey.$hash;
$hash=$hash.checksum($hash);
$hash=my_base_convert($hash,
16, '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz');
$hash=str_pad($hash, 34, '1', STR_PAD_LEFT);
return $hash;
}
?>

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings