filecoin-project / filecoin-project/go-address
Input validation on SECP256K1 address generation
- Dominant language
- Go
- Stars
- 16
- Forks
- 25
- PR merge metrics
- No merged PRs in 30d
Description
**Background**
I didn't appreciate that the blake2b hashing algorithm will hash _any_ data to produce an otherwise valid looking `f1...` SECP256k1 filecoin address.
While stepping through the process, I used a compressed SECP256k1 key (33 bytes starting with `02` or `03`) as input and got a valid looking address as output, and it took me some time to backtrack and realise my mistake :)
This code looks like it is doing some input validation:
https://github.com/filecoin-project/go-address/blob/5769c0de15a02ef38055aeb277ff159db822c5bc/address.go#L225-L228
ensuring the resulting hash is 20 bytes long (`PayloadHashLength`), but that is set by the config passed to the `hash()` function https://github.com/filecoin-project/go-address/blob/5769c0de15a02ef38055aeb277ff159db822c5bc/constants.go#L65 , so will _always_ produce a 20 byte result, unless the code is changed. There is no way a user can trigger this validation via input.
This is academic, but I'm interested to know whether all possible hashed data producing `f1...` filecoin address will always have a corresponding uncompressed SECP256k1 public key (65 bytes beginning with `04`), or whether some otherwise valid addresses are impossible to access. (i.e. there is no key that could hash to it). Are there enough hash collisions in blake2b to allow this?
**Suggestions**
In order to protect others from falling into the same trap and generating unintentionally inaccessible addresses:
* Add some input validation on SECP256k1 address generation to reject any data which is not an uncompressed SECP256k1 public key, i.e. _exactly_ 65 byte beginning with an 04. There doesn't seem to be any sensible way to pad or infer a correct key from any other data payload (that I can see). There doesn't seem to be any valid reason to run the algorithm with data other than a valid uncompressed public key -- it's most likely to be an error. (experimenting with blake2b hash collisions is the only reason I can think of ...).
* Add / improve documentation on the `cmd/fcaddr` example tool to make it clear that a valid uncompressed SECP256k1 public key is required to get the expected behaviour. The current messaging "converting Secp256k1 to address" is wrong if the user didn't provide Secp256k1 data. It is simply converting _any_ provided data to an `f1...` style address by blake2b hashing.
The command line tool _could_ accept compressed SECP256k1 public keys to be helpful, but I'm not sure it's worth adding that logic so long as the uncompressed requirement is clear. This is a filecoin utility, not a secp256k1 util.
Please let me know if I'm missing anything with the above. I'm still learning about Filecoin and trying to help others from my mistakes.
**Examples for testing**
I believe this is the lowest sensible input value: (uncompressed public key representing 0,0 on the secp265k1 curve, in hex)
```
./fcaddr -type=secp256k1 <<< 0400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
```
Examples of invalid, non-SECP256K1 input, producing otherwise valid addresses:
```
./fcaddr -type=secp256k1 <<< 00
f1bavntex3o2drym5bxgmtucbjkl7kzjpguc7yuzq
```
```
./fcaddr -type=secp256k1 <<< 0000
f1mtlotjg4lohjmvwvxubbrxqyspatkel7lzgzb2y
```
Contributor guide
Assessment
This issue has not been assessed yet.