go-libp2p icon indicating copy to clipboard operation
go-libp2p copied to clipboard

core: can't decode an encoded peer.ID, for certain peer ID values

Open marten-seemann opened this issue 4 years ago • 4 comments

Minimal example:

package main

import (
	"log"

	"github.com/libp2p/go-libp2p-core/peer"
)

func main() {
	data := []byte{
		0x00, 0x01, 0x72, 0x94, 0xe4, 0xc5, 0x30, 0x1b, 0x30, 0x30, 0x30, 0x0e, 0x30, 0xbd, 0xef,
		0x30, 0x30, 0x30, 0x22, 0x77, 0xff, 0x22, 0x30, 0xef, 0x30, 0x30, 0x30, 0xbd, 0x30, 0x30, 0x30,
		0x30, 0xbd, 0x30, 0x30,
	}
	var id peer.ID
	if err := id.UnmarshalText(data); err != nil {
		log.Fatal("unmarshal failed")
	}
	encoded := peer.Encode(id)
	id2, err := peer.Decode(encoded)
	if err != nil {
		log.Fatal(err)
	}
	if id != id2 {
		log.Fatal("expected ids to match")
	}
}

The peer.Decode(encoded) step here fails: failed to parse peer ID: expected 1 as the cid version number, got: 3261359138256.

As far as I can tell, the reason for the failure is that Encode always uses base58 encoding: https://github.com/libp2p/go-libp2p-core/blob/83ac1d370dfe3f04aee38a904f73e49070be4863/peer/peer.go#L144-L146 Encoding uses the CID code to decode, if the encoded value doesn't start with "1" or "Qm": https://github.com/libp2p/go-libp2p-core/blob/83ac1d370dfe3f04aee38a904f73e49070be4863/peer/peer.go#L172-L187

I'm not sure what to do here. Can someone with more IPLD experience help me out here? @rvagg, @warpfork?

marten-seemann avatar Feb 13 '21 13:02 marten-seemann

@marten-seemann what are you trying to do here and how did you generate the bytes in data? You're taking a bunch of bytes (data) and passing them to a function that expects a string id.UnmarshalText(data) so that seems super likely to fail.

Note: the format for peerIDs is here https://github.com/libp2p/specs/blob/d4321b94b51380a371a8f25eb82e11105afbfceb/peer-ids/peer-ids.md, and while peerIDs can be represented as a bash58 multihash or as a CIDv1 with the libp2p-key codec there is currently only one valid binary representation.

aschmahmann avatar Feb 14 '21 00:02 aschmahmann

@aschmahmann data was generated by a fuzzer. I wouldn't be suprised if UnmarshalText failed, but in this case it doesn't. UnmarshalText suceeds, giving me a seemingly valid peer.ID. I then use peer.Encode to encode this peer ID and then try to decode it using peer.Decode. It's this last step that fails.

I would assume that encoding and then decoding a peer.ID should always succeed, no matter what the value of the peer.ID, and that I arrive at the same value that I started with. Is that not correct?

marten-seemann avatar Feb 14 '21 01:02 marten-seemann

I think this is one of those things that sits on the performance <-> strictness spectrum.

FromCid https://github.com/libp2p/go-libp2p-core/blob/83ac1d370dfe3f04aee38a904f73e49070be4863/peer/peer.go#L198 which is internally called by peer.Decode and id.UnmarshalText should probably check if the multihash is SHA2-256 or Identity (the only ones allowed by the spec) and fail otherwise.

Side Note: It seems to me like all the functions in https://github.com/libp2p/go-libp2p-core/blob/43f10f228825a8ed0d195b0f8e8de82dc0c594b1/peer/peer_serde.go should be using Encode/Decode instead of the b58 versions since those are deprecated, unless there's some particular reason we want to be using b58.

aschmahmann avatar Feb 14 '21 02:02 aschmahmann

Yeah, this is a bug in decode.

Stebalien avatar Feb 15 '21 16:02 Stebalien

A smaller input that triggers the same bug (found via https://github.com/google/oss-fuzz/pull/9202)

package main

import (
	"log"
	"github.com/libp2p/go-libp2p/core/peer"
)

func main() {
	var data string = "\x00\x01r0\x010"
	var id peer.ID
	if err := id.UnmarshalText([]byte(data)); err != nil {
		log.Print("Unmarshal failed")
		return
	}
	encoded := peer.Encode(id)
	id2, err := peer.Decode(encoded)
	if err != nil {
		log.Fatal(err)
	}
	if id != id2 {
		log.Fatal("expected ids to match")
	}
}

bshastry avatar Dec 15 '22 10:12 bshastry