Cherry-pick #4483 from scionproto/scion

pkg/slayers/path/scion/BUILD.bazel

@@ -24,6 +24,7 @@ go_test(
     ],
     deps = [
         ":go_default_library",
+        "//pkg/private/serrors:go_default_library",
         "//pkg/slayers/path:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
         "@com_github_stretchr_testify//require:go_default_library",

pkg/slayers/path/scion/base.go

@@ -27,10 +27,17 @@ import (
 	//@ sl "github.com/scionproto/scion/verification/utils/slices"
 )
 
-// MetaLen is the length of the PathMetaHeader.
-const MetaLen = 4
+const (
+	// MaxINFs is the maximum number of info fields in a SCION path.
+	MaxINFs = 3
+	// MaxHops is the maximum number of hop fields in a SCION path.
+	MaxHops = 64
 
-const PathType path.Type = 1
+	// MetaLen is the length of the PathMetaHeader.
+	MetaLen = 4
+
+	PathType path.Type = 1
+)
 
 // @ requires path.PathPackageMem()
 // @ requires !path.Registered(PathType)
@@ -142,6 +149,14 @@ func (s *Base) DecodeFromBytes(data []byte) (r error) {
 		//@ assume int(s.PathMeta.SegLen[i]) >= 0
 		s.NumHops += int(s.PathMeta.SegLen[i])
 	}
+
+	// We must check the validity of NumHops. It is possible to fit more than 64 hops in
+	// the length of a scion header. Yet a path of more than 64 hops cannot be followed to
+	// the end because CurrHF is only 6 bits long.
+	if s.NumHops > MaxHops {
+		//@ defer fold s.NonInitMem()
+		return serrors.New("NumHops too large", "NumHops", s.NumHops, "Maximum", MaxHops)
+	}
 	//@ fold s.Mem()
 	return nil
 }
@@ -212,7 +227,9 @@ func (s *Base) infIndexForHF(hf uint8) (r uint8) {
 	}
 }
 
-// Len returns the length of the path in bytes.
+// Len returns the length of the path in bytes. That is, the number of byte required to
+// store it, based on the metadata. The actual number of bytes available to contain it
+// can be inferred from the common header field HdrLen. It may or may not be consistent.
 // @ pure
 // @ requires acc(s.Mem(), _)
 // @ ensures  r >= MetaLen

pkg/slayers/path/scion/decoded.go

@@ -24,13 +24,6 @@ import (
 	//@ sl "github.com/scionproto/scion/verification/utils/slices"
 )
 
-const (
-	// MaxINFs is the maximum number of info fields in a SCION path.
-	MaxINFs = 3
-	// MaxHops is the maximum number of hop fields in a SCION path.
-	MaxHops = 64
-)
-
 // Decoded implements the SCION (data-plane) path type. Decoded is intended to be used in
 // non-performance critical code paths, where the convenience of having a fully parsed path trumps
 // the loss of performance.

pkg/slayers/path/scion/raw_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/scionproto/scion/pkg/private/serrors"
 	"github.com/scionproto/scion/pkg/slayers/path"
 	"github.com/scionproto/scion/pkg/slayers/path/scion"
 )
@@ -51,6 +52,19 @@ var emptyRawTestPath = &scion.Raw{
 	Raw: make([]byte, scion.MetaLen),
 }
 
+var overlongPath = &scion.Raw{
+	Base: scion.Base{
+		PathMeta: scion.MetaHdr{
+			CurrINF: 0,
+			CurrHF:  0,
+			SegLen:  [3]uint8{24, 24, 17},
+		},
+		NumINF:  3,
+		NumHops: 65,
+	},
+	Raw: rawPath,
+}
+
 func TestRawSerialize(t *testing.T) {
 	b := make([]byte, rawTestPath.Len())
 	assert.NoError(t, rawTestPath.SerializeTo(b))
@@ -63,14 +77,23 @@ func TestRawDecodeFromBytes(t *testing.T) {
 	assert.Equal(t, rawTestPath, s)
 }
 
-func TestRawSerliazeDecode(t *testing.T) {
+func TestRawSerializeDecode(t *testing.T) {
 	b := make([]byte, rawTestPath.Len())
 	assert.NoError(t, rawTestPath.SerializeTo(b))
 	s := &scion.Raw{}
 	assert.NoError(t, s.DecodeFromBytes(b))
 	assert.Equal(t, rawTestPath, s)
 }
 
+func TestOverlongSerializeDecode(t *testing.T) {
+	b := make([]byte, overlongPath.Len())
+	assert.NoError(t, overlongPath.SerializeTo(b)) // permitted, if only to enable this test.
+	s := &scion.Raw{}
+	expected := serrors.New("NumHops too large", "NumHops", 65, "Maximum", scion.MaxHops)
+	err := s.DecodeFromBytes(b)
+	assert.Equal(t, expected.Error(), err.Error())
+}
+
 func TestRawReverse(t *testing.T) {
 	for name, tc := range pathReverseCases {
 		name, tc := name, tc