Skip to content

Commit

Permalink
lib/protocol: Add some consistency checks on incoming index updates (f…
Browse files Browse the repository at this point in the history
…ixes syncthing#4053)

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

GitHub-Pull-Request: syncthing#4064
  • Loading branch information
calmh authored and AudriusButkevicius committed Mar 27, 2017
1 parent 1ad547f commit b75b419
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 4 deletions.
35 changes: 31 additions & 4 deletions lib/protocol/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
54 changes: 54 additions & 0 deletions lib/protocol/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

0 comments on commit b75b419

Please sign in to comment.