gagliardetto / solana-go

Go SDK library and RPC client for the Solana Blockchain
Apache License 2.0
817 stars 241 forks source link

Not return `err` when pass `random str or invalid privateKey` to `func PrivateKeyFromBase58` #232

Open Almazatun opened 2 months ago

Almazatun commented 2 months ago

I just tried to use the func to validate private key but got some other behavior that func.

https://github.com/gagliardetto/solana-go/blob/main/keys.go#L48

It's correct behavior ?

Almazatun commented 1 month ago

Also, I want to add some notes if pass incorrect value or random str the func and call public method PublicKey() or call Sign(pk) not return error argument but call panic. But the methods should return err argument.

panic: runtime error: slice bounds out of range [x:x] | panic: ed25519: bad private key length

If pass correct privateKey, everything will be fine and return publicKey.

pikomonde commented 1 month ago

I just tried to use the func to validate private key but got some other behavior that func.

https://github.com/gagliardetto/solana-go/blob/main/keys.go#L48

It's correct behavior ?

Hi @Almazatun , can you explain what is the "some other behaviour"?

Also, I want to add some notes if pass incorrect value or random str the func and call public method PublicKey() or call Sign(pk) not return error argument but call panic. But the methods should return err argument.

panic: runtime error: slice bounds out of range [x:x] | panic: ed25519: bad private key length

If pass correct privateKey, everything will be fine and return publicKey.

Can you give the sample of the code that give that exact error? It will help us to debugging. Thank you

Almazatun commented 1 month ago

@pikomonde Thank you very much for your reply. The PrivateKeyFromBase58 method returns two values. If I guess that means if pass invalid privateKey should be returned err that does not equal nil. But instead of this behavior, you will get err = nil.

invalidPrivateKey := "6HsFaXKVD7mo43oTbdqyGgAnYFeNNhqY75B3JGJ6K8a227KjjG3uW3v"

_, err := solana.PrivateKeyFromBase58(invalidPk)

fmt.Println(err)

Is this the correct behavior of the method?

pikomonde commented 1 month ago

Ah, I got your question now. My question is, how do you know that 6HsFaXKVD7mo43oTbdqyGgAnYFeNNhqY75B3JGJ6K8a227KjjG3uW3v is an invalid private key? Is there any docs on Solana that classify that this particular private key is invalid?

Almazatun commented 1 month ago

@pikomonde You can copy and paste it to phantom wallet for checking.

pikomonde commented 1 month ago

I'm not sure if it is the correct behaviour or not (I think we need to compare with solana's official client)

But I think it is invalid because it is not on the a valid "ed25519" pair, and unfortunately the golang's "crypto/ed25519" package not providing a valid validation for it. Need to implement using this: https://security.stackexchange.com/questions/233099/validating-an-ed25519-public-key (haven't check it yet)

So I think there are 2 action items:

pikomonde commented 1 month ago

Just realized that there is already a function provided by this library:

solana.IsOnCurve("6HsFaXKVD7mo43oTbdqyGgAnYFeNNhqY75B3JGJ6K8a227KjjG3uW3v")