Missing length check in recvPacket()

[vansante]

I got this panic yesterday:

goroutine 33156586 [running]:
runtime/debug.Stack(0xe7d260, 0xc002d11238, 0xf7680c)
    /usr/local/go/src/runtime/debug/stack.go:24 +0x9f
panic(0xf0c700, 0xc001713020)
    /usr/local/go/src/runtime/panic.go:969 +0x1b9
github.com/pkg/sftp.recvPacket(0x10e9040, 0xc002e7a580, 0x0, 0x8ac9, 0x10f4e00, 0xc00791a0c0, 0x0, 0x0, 0x0, 0x0)
    /go/pkg/mod/github.com/pkg/sftp@v1.12.0/packet.go:169 +0x33b
github.com/pkg/sftp.(*conn).recvPacket(...)
    /go/pkg/mod/github.com/pkg/sftp@v1.12.0/conn.go:26
github.com/pkg/sftp.(*RequestServer).serveLoop(0xc003e0d100, 0xc030c93200, 0x0, 0x0)
    /go/pkg/mod/github.com/pkg/sftp@v1.12.0/request-server.go:118 +0xeb
github.com/pkg/sftp.(*RequestServer).Serve(0xc003e0d100, 0x0, 0x0)
    /go/pkg/mod/github.com/pkg/sftp@v1.12.0/request-server.go:161 +0x169
app/daemon/sftp.(*SSHConn).serviceSFTPChannel.func2(0xc00ec12c00, 0xc003e0d100, 0x1105e80, 0xc0033a83c0, 0xc008a175c0)
    /builds/app/daemon/sftp/ssh_conn.go:177 +0xb1
created by app/daemon/sftp.(*SSHConn).serviceSFTPChannel
    /builds/app/daemon/sftp/ssh_conn.go:173 +0x2d2

The error is in the function recvPacket():

func recvPacket(r io.Reader, alloc *allocator, orderID uint32) (uint8, []byte, error) {
    var b []byte
    if alloc != nil {
        b = alloc.GetPage(orderID)
    } else {
        b = make([]byte, 4)
    }
    if _, err := io.ReadFull(r, b[:4]); err != nil {
        return 0, nil, err
    }
    length, _ := unmarshalUint32(b)
    if length > maxMsgLength {
        debug("recv packet %d bytes too long", length)
        return 0, nil, errLongPacket
    }
    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)
        return 0, nil, err
    }
    if debugDumpRxPacketBytes {
        debug("recv packet: %s %d bytes %x", fxp(b[0]), length, b[1:length])
    } else if debugDumpRxPacket {
        debug("recv packet: %s %d bytes", fxp(b[0]), length)
    }
    return b[0], b[1:length], nil
}

It checks for maximum message length on line 153, but not whether the buffer b is at least one byte, thus making the very last return operation panic.

It should be relatively simple to add a check for the minimum length of 1 prior to slicing. If desired, I can make a PR for this.

[drakkan]

Hi,

I think this is already fixed in master

https://github.com/pkg/sftp/blob/master/packet.go#L157