Added trailing bytes guard to ASN.1 decoding

Signed-off-by: Effi-S <effi.szt@gmail.com>

Author: Effi-S

.github/workflows/nightly-fuzz.yml

@@ -111,6 +111,12 @@ jobs:
           - name: storage-sql-table-names
             pkg: ./token/services/storage/db/sql/common
             func: FuzzGetTableNamesNoPanic
+          - name: tokens-typed-token-unmarshal
+            pkg: ./token/services/tokens
+            func: FuzzUnmarshalTypedTokenNoPanic
+          - name: tokens-typed-metadata-unmarshal
+            pkg: ./token/services/tokens
+            func: FuzzUnmarshalTypedMetadataNoPanic
           - name: driver-identity-configuration-unique-id
             pkg: ./token/driver
             func: FuzzIdentityConfigurationUniqueIDInjective

token/services/tokens/typed.go

@@ -30,10 +30,13 @@ func (i TypedToken) Bytes() ([]byte, error) {
 // UnmarshalTypedToken deserializes an ASN.1 encoded byte slice into a TypedToken structure.
 func UnmarshalTypedToken(token driver.Token) (*TypedToken, error) {
 	si := &TypedToken{}
-	_, err := asn1.Unmarshal(token, si)
+	rest, err := asn1.Unmarshal(token, si)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to unmarshal to TypedToken")
 	}
+	if len(rest) != 0 {
+		return nil, errors.Errorf("trailing bytes after TypedToken")
+	}
 
 	return si, nil
 }
@@ -64,10 +67,13 @@ func (i TypedMetadata) Bytes() ([]byte, error) {
 // UnmarshalTypedMetadata deserializes an ASN.1 encoded byte slice into a TypedMetadata structure.
 func UnmarshalTypedMetadata(metadata driver.Metadata) (*TypedMetadata, error) {
 	si := &TypedMetadata{}
-	_, err := asn1.Unmarshal(metadata, si)
+	rest, err := asn1.Unmarshal(metadata, si)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to unmarshal to TypedMetadata")
 	}
+	if len(rest) != 0 {
+		return nil, errors.Errorf("trailing bytes after TypedMetadata")
+	}
 
 	return si, nil
 }

token/services/tokens/typed_test.go

@@ -15,6 +15,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const maxFuzzTypedBytes = 256 << 10
+
 func TestSerialization(t *testing.T) {
 	raw := []byte("pineapple")
 	wrappedToken, err := tokens.WrapWithType(0, raw)
@@ -24,3 +26,84 @@ func TestSerialization(t *testing.T) {
 	assert.Equal(t, driver.Type(0), tok.Type)
 	assert.Equal(t, driver.Token(raw), tok.Token)
 }
+
+// TestUnmarshalTypedTokenRejectsTrailingBytes verifies that a valid encoding
+// with arbitrary junk appended is rejected rather than silently accepted. See
+// issue #2189: ignoring the ASN.1 `rest` slice makes the decoder malleable —
+// two distinct byte strings would otherwise decode to the same object.
+func TestUnmarshalTypedTokenRejectsTrailingBytes(t *testing.T) {
+	valid, err := tokens.WrapWithType(7, driver.Token("payload"))
+	require.NoError(t, err)
+
+	// Sanity: the clean encoding still decodes fine.
+	_, err = tokens.UnmarshalTypedToken(valid)
+	require.NoError(t, err)
+
+	withTrailer := append(append([]byte{}, valid...), 0x00, 0x01, 0x02)
+	got, err := tokens.UnmarshalTypedToken(withTrailer)
+	require.Error(t, err)
+	require.Nil(t, got)
+	require.Contains(t, err.Error(), "trailing bytes")
+}
+
+// TestUnmarshalTypedMetadataRejectsTrailingBytes is the metadata counterpart of
+// TestUnmarshalTypedTokenRejectsTrailingBytes.
+func TestUnmarshalTypedMetadataRejectsTrailingBytes(t *testing.T) {
+	valid, err := tokens.WrapMetadataWithType(7, driver.Metadata("payload"))
+	require.NoError(t, err)
+
+	_, err = tokens.UnmarshalTypedMetadata(valid)
+	require.NoError(t, err)
+
+	withTrailer := append(append([]byte{}, valid...), 0x00, 0x01, 0x02)
+	got, err := tokens.UnmarshalTypedMetadata(withTrailer)
+	require.Error(t, err)
+	require.Nil(t, got)
+	require.Contains(t, err.Error(), "trailing bytes")
+}
+
+// FuzzUnmarshalTypedTokenNoPanic fuzzes UnmarshalTypedToken with arbitrary
+// bytes. This decoder sits on the receive path for untrusted, ledger-stored
+// and peer-supplied token bytes (see issue #2189), so any panic here is an
+// unauthenticated DoS against every caller that reads typed tokens.
+func FuzzUnmarshalTypedTokenNoPanic(f *testing.F) {
+	valid, err := tokens.WrapWithType(7, driver.Token("payload"))
+	require.NoError(f, err)
+
+	f.Add([]byte(valid))
+	f.Add([]byte{})
+	f.Add([]byte("malformed"))
+	f.Add([]byte(valid)[:len(valid)/2])
+	f.Add(append(append([]byte{}, valid...), 0x00, 0x01, 0x02))
+
+	f.Fuzz(func(t *testing.T, raw []byte) {
+		if len(raw) > maxFuzzTypedBytes {
+			t.Skip()
+		}
+		require.NotPanics(t, func() {
+			_, _ = tokens.UnmarshalTypedToken(raw)
+		})
+	})
+}
+
+// FuzzUnmarshalTypedMetadataNoPanic fuzzes UnmarshalTypedMetadata with
+// arbitrary bytes, for the same reasons as FuzzUnmarshalTypedTokenNoPanic.
+func FuzzUnmarshalTypedMetadataNoPanic(f *testing.F) {
+	valid, err := tokens.WrapMetadataWithType(7, driver.Metadata("payload"))
+	require.NoError(f, err)
+
+	f.Add([]byte(valid))
+	f.Add([]byte{})
+	f.Add([]byte("malformed"))
+	f.Add([]byte(valid)[:len(valid)/2])
+	f.Add(append(append([]byte{}, valid...), 0x00, 0x01, 0x02))
+
+	f.Fuzz(func(t *testing.T, raw []byte) {
+		if len(raw) > maxFuzzTypedBytes {
+			t.Skip()
+		}
+		require.NotPanics(t, func() {
+			_, _ = tokens.UnmarshalTypedMetadata(raw)
+		})
+	})
+}