CBG-3238 perform a length check on raw bytes (#6438)

* CBG-3238 perform a length check on raw bytes

- refactor attachment compaction tests so we can make sure that it doesn't terminate

* Add comment

* remove redundant returns

* Move error handling into getAttachmentSyncData

* PR fixup

- remove attachment compaction changes and leave only a change to avoid
  failure to mark changes if xattr length is invalid. Treat this as a
  document without xattrs in order to not fail attachment compaction
  test.
- add code to prevent overflow if xattr len is a number between 1-4
  which would cause a panic.

* Update tests

* remove test tested in separate function

base/error.go

@@ -50,6 +50,9 @@ var (
 	// ErrXattrNotFound is returned if a requested xattr is not present on a DCP event
 	ErrXattrNotFound = &sgError{"Xattr Not Found"}
 
+	// ErrXattrInvalidLen is returned if the xattr is corrupt.
+	ErrXattrInvalidLen = &sgError{"Xattr stream length"}
+
 	// ErrPartialViewErrors is returned if the view call contains any partial errors.
 	// This is more of a warning, and inspecting ViewResult.Errors is required for detail.
 	ErrPartialViewErrors = &sgError{"Partial errors in view"}

db/attachment_compaction.go

@@ -211,7 +211,7 @@ func getAttachmentSyncData(dataType uint8, data []byte) (*AttachmentCompactionDa
 	if dataType&base.MemcachedDataTypeXattr != 0 {
 		body, xattr, _, err := parseXattrStreamData(base.SyncXattrName, "", data)
 		if err != nil {
-			if errors.Is(err, base.ErrXattrNotFound) {
+			if errors.Is(err, base.ErrXattrNotFound) || errors.Is(err, base.ErrXattrInvalidLen) {
 				return nil, nil
 			}
 			return nil, err

db/change_cache_test.go

@@ -2255,3 +2255,12 @@ func BenchmarkDocChanged(b *testing.B) {
 		})
 	}
 }
+
+func TestInvalidXattrStream(t *testing.T) {
+
+	body, xattr, userXattr, err := parseXattrStreamData(base.SyncXattrName, "", []byte("abcde"))
+	require.Error(t, err)
+	require.Nil(t, body)
+	require.Nil(t, xattr)
+	require.Nil(t, userXattr)
+}

db/document.go

@@ -515,6 +515,9 @@ func parseXattrStreamData(xattrName string, userXattrName string, data []byte) (
 	}
 
 	xattrsLen := binary.BigEndian.Uint32(data[0:4])
+	if int(xattrsLen+4) > len(data) {
+		return nil, nil, nil, fmt.Errorf("%w (%d) from bytes %+v", base.ErrXattrInvalidLen, xattrsLen, data[0:4])
+	}
 	body = data[xattrsLen+4:]
 	if xattrsLen == 0 {
 		return body, nil, nil, nil

db/document_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/couchbase/sync_gateway/base"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TODO: Could consider checking this in as a file and include it into the compiled test binary using something like https://github.com/jteeuwen/go-bindata
@@ -291,3 +292,61 @@ func TestGetDeepMutableBody(t *testing.T) {
 		})
 	}
 }
+
+func TestInvalidXattrStreamDataLen(t *testing.T) {
+	testCases := []struct {
+		name        string
+		body        []byte
+		expectedErr error
+	}{
+		{
+			name:        "bad value",
+			body:        []byte("abcde"),
+			expectedErr: base.ErrXattrInvalidLen,
+		},
+		{
+			name:        "xattr length 4, overflow",
+			body:        []byte{0x00, 0x00, 0x00, 0x04, 0x01},
+			expectedErr: base.ErrXattrInvalidLen,
+		},
+	}
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			// parseXattrStreamData is the underlying function
+			body, xattr, userXattr, err := parseXattrStreamData(base.SyncXattrName, "", test.body)
+			require.Error(t, err)
+			require.ErrorIs(t, err, test.expectedErr)
+			require.Nil(t, body)
+			require.Nil(t, xattr)
+			require.Nil(t, userXattr)
+			// UnmarshalDocumentSyncData wraps parseXattrStreamData
+			result, rawBody, rawXattr, rawUserXattr, err := UnmarshalDocumentSyncDataFromFeed(test.body, base.MemcachedDataTypeXattr, "", false)
+			require.ErrorIs(t, err, base.ErrXattrInvalidLen)
+			require.Nil(t, result)
+			require.Nil(t, rawBody)
+			require.Nil(t, rawXattr)
+			require.Nil(t, rawUserXattr)
+
+		})
+	}
+}
+
+func TestInvalidXattrStreamEmptyBody(t *testing.T) {
+	inputStream := []byte{0x00, 0x00, 0x00, 0x01, 0x01}
+	emptyBody := []byte{}
+	// parseXattrStreamData is the underlying function
+	body, xattr, userXattr, err := parseXattrStreamData(base.SyncXattrName, "", inputStream)
+	require.NoError(t, err)
+	require.Equal(t, emptyBody, body)
+	require.Nil(t, xattr)
+	require.Nil(t, userXattr)
+
+	// UnmarshalDocumentSyncData wraps parseXattrStreamData
+	result, rawBody, rawXattr, rawUserXattr, err := UnmarshalDocumentSyncDataFromFeed(inputStream, base.MemcachedDataTypeXattr, "", false)
+	require.Error(t, err) // unexpected end of JSON input
+	require.Nil(t, result)
+	require.Equal(t, emptyBody, rawBody)
+	require.Nil(t, rawXattr)
+	require.Nil(t, rawUserXattr)
+
+}