From b75b4190c8b61e32c849857750f2d0ff51712f51 Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Mon, 27 Mar 2017 07:21:08 +0000
Subject: [PATCH] lib/protocol: Add some consistency checks on incoming index
 updates (fixes #4053)

With this change we will throw a protocol error on some kinds of
malformed index entries.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4064
---
 lib/protocol/protocol.go      | 35 ++++++++++++++++++++---
 lib/protocol/protocol_test.go | 54 +++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go
index 954a8df5b..da089baa3 100644
--- a/lib/protocol/protocol.go
+++ b/lib/protocol/protocol.go
@@ -57,6 +57,9 @@ var (
 	errUnknownMessage       = errors.New("unknown message")
 	errInvalidFilename      = errors.New("filename is invalid")
 	errUncleanFilename      = errors.New("filename not in canonical format")
+	errDeletedHasBlocks     = errors.New("deleted file with non-empty block list")
+	errDirectoryHasBlocks   = errors.New("directory with non-empty block list")
+	errFileHasNoBlocks      = errors.New("file with empty block list")
 )
 
 type Model interface {
@@ -308,7 +311,7 @@ func (c *rawConnection) readerLoop() (err error) {
 			if state != stateReady {
 				return fmt.Errorf("protocol error: index message in state %d", state)
 			}
-			if err := checkFilenames(msg.Files); err != nil {
+			if err := checkIndexConsistency(msg.Files); err != nil {
 				return fmt.Errorf("protocol error: index: %v", err)
 			}
 			c.handleIndex(*msg)
@@ -319,7 +322,7 @@ func (c *rawConnection) readerLoop() (err error) {
 			if state != stateReady {
 				return fmt.Errorf("protocol error: index update message in state %d", state)
 			}
-			if err := checkFilenames(msg.Files); err != nil {
+			if err := checkIndexConsistency(msg.Files); err != nil {
 				return fmt.Errorf("protocol error: index update: %v", err)
 			}
 			c.handleIndexUpdate(*msg)
@@ -466,15 +469,39 @@ func (c *rawConnection) handleIndexUpdate(im IndexUpdate) {
 	c.receiver.IndexUpdate(c.id, im.Folder, im.Files)
 }
 
-func checkFilenames(fs []FileInfo) error {
+// checkIndexConsistency verifies a number of invariants on FileInfos received in
+// index messages.
+func checkIndexConsistency(fs []FileInfo) error {
 	for _, f := range fs {
-		if err := checkFilename(f.Name); err != nil {
+		if err := checkFileInfoConsistency(f); err != nil {
 			return fmt.Errorf("%q: %v", f.Name, err)
 		}
 	}
 	return nil
 }
 
+// checkFileInfoConsistency verifies a number of invariants on the given FileInfo
+func checkFileInfoConsistency(f FileInfo) error {
+	if err := checkFilename(f.Name); err != nil {
+		return err
+	}
+
+	switch {
+	case f.Deleted && len(f.Blocks) != 0:
+		// Deleted files should have no blocks
+		return errDeletedHasBlocks
+
+	case f.Type == FileInfoTypeDirectory && len(f.Blocks) != 0:
+		// Directories should have no blocks
+		return errDirectoryHasBlocks
+
+	case !f.Deleted && f.Type == FileInfoTypeFile && len(f.Blocks) == 0:
+		// Non-deleted files should have at least one block
+		return errFileHasNoBlocks
+	}
+	return nil
+}
+
 // checkFilename verifies that the given filename is valid according to the
 // spec on what's allowed over the wire. A filename failing this test is
 // grounds for disconnecting the device.
diff --git a/lib/protocol/protocol_test.go b/lib/protocol/protocol_test.go
index dc2e35367..f19710ba9 100644
--- a/lib/protocol/protocol_test.go
+++ b/lib/protocol/protocol_test.go
@@ -346,3 +346,57 @@ func TestCheckFilename(t *testing.T) {
 		}
 	}
 }
+
+func TestCheckConsistency(t *testing.T) {
+	cases := []struct {
+		fi FileInfo
+		ok bool
+	}{
+		{
+			// valid
+			fi: FileInfo{
+				Name:   "foo",
+				Type:   FileInfoTypeFile,
+				Blocks: []BlockInfo{{Size: 1234, Offset: 0, Hash: []byte{1, 2, 3, 4}}},
+			},
+			ok: true,
+		},
+		{
+			// deleted with blocks
+			fi: FileInfo{
+				Name:    "foo",
+				Deleted: true,
+				Type:    FileInfoTypeFile,
+				Blocks:  []BlockInfo{{Size: 1234, Offset: 0, Hash: []byte{1, 2, 3, 4}}},
+			},
+			ok: false,
+		},
+		{
+			// no blocks
+			fi: FileInfo{
+				Name: "foo",
+				Type: FileInfoTypeFile,
+			},
+			ok: false,
+		},
+		{
+			// directory with blocks
+			fi: FileInfo{
+				Name:   "foo",
+				Type:   FileInfoTypeDirectory,
+				Blocks: []BlockInfo{{Size: 1234, Offset: 0, Hash: []byte{1, 2, 3, 4}}},
+			},
+			ok: false,
+		},
+	}
+
+	for _, tc := range cases {
+		err := checkFileInfoConsistency(tc.fi)
+		if tc.ok && err != nil {
+			t.Errorf("Unexpected error %v (want nil) for %v", err, tc.fi)
+		}
+		if !tc.ok && err == nil {
+			t.Errorf("Unexpected nil error for %v", tc.fi)
+		}
+	}
+}