Don't crash when the packet length is zero

Also, report EOF in the middle of a packet as a protocol error rather
than EOF.

client_test.go

@@ -1,6 +1,7 @@
 package sftp
 
 import (
+	"bytes"
 	"errors"
 	"io"
 	"os"
@@ -208,3 +209,21 @@ func TestUseFstatChecked(t *testing.T) {
 	testFstatOption(t, UseFstat(true), true)
 	testFstatOption(t, UseFstat(false), false)
 }
+
+type sink struct{}
+
+func (*sink) Close() error                { return nil }
+func (*sink) Write(p []byte) (int, error) { return len(p), nil }
+
+func TestClientWithBrokenServer(t *testing.T) {
+	// Packet length zero (never valid). This used to crash the client.
+	packet := []byte{0, 0, 0, 0}
+	w := &sink{}
+
+	r := bytes.NewReader(packet)
+	c, err := NewClientPipe(r, w)
+	if err != errShortPacket {
+		t.Error("expected errShortPacket, got", err)
+	}
+	c.Close()
+}

fuzz.go

@@ -0,0 +1,22 @@
+// +build gofuzz
+
+package sftp
+
+import "bytes"
+
+type sink struct{}
+
+func (*sink) Close() error                { return nil }
+func (*sink) Write(p []byte) (int, error) { return len(p), nil }
+
+var devnull = &sink{}
+
+// To run: go-fuzz-build && go-fuzz
+func Fuzz(data []byte) int {
+	c, err := NewClientPipe(bytes.NewReader(data), devnull)
+	if err != nil {
+		return 0
+	}
+	c.Close()
+	return 1
+}

packet.go

@@ -154,11 +154,18 @@ func recvPacket(r io.Reader, alloc *allocator, orderID uint32) (uint8, []byte, e
 		debug("recv packet %d bytes too long", length)
 		return 0, nil, errLongPacket
 	}
+	if length == 0 {
+		debug("recv packet of %d bytes too short", length)
+		return 0, nil, errShortPacket
+	}
 	if alloc == nil {
 		b = make([]byte, length)
 	}
 	if _, err := io.ReadFull(r, b[:length]); err != nil {
 		debug("recv packet %d bytes: err %v", length, err)
+		if err == io.EOF || err == io.ErrUnexpectedEOF {
+			err = errShortPacket
+		}
 		return 0, nil, err
 	}
 	if debugDumpRxPacketBytes {

server_test.go

@@ -1,6 +1,7 @@
 package sftp
 
 import (
+	"bytes"
 	"io"
 	"os"
 	"path"
@@ -365,3 +366,31 @@ func TestStatNonExistent(t *testing.T) {
 		}
 	}
 }
+
+func TestServerWithBrokenClient(t *testing.T) {
+	validInit := sp(sshFxInitPacket{Version: 3})
+	brokenOpen := sp(sshFxpOpenPacket{Path: "foo"})
+	brokenOpen = brokenOpen[:len(brokenOpen)-2]
+
+	for _, clientInput := range [][]byte{
+		// Packet length zero (never valid). This used to crash the server.
+		{0, 0, 0, 0},
+		append(validInit, 0, 0, 0, 0),
+
+		// Client hangs up mid-packet.
+		append(validInit, brokenOpen...),
+	} {
+		srv, err := NewServer(struct {
+			io.Reader
+			io.WriteCloser
+		}{
+			bytes.NewReader(clientInput),
+			&sink{},
+		})
+		assert.NoError(t, err)
+
+		err = srv.Serve()
+		assert.Error(t, err)
+		srv.Close()
+	}
+}