Panic when sending large data

[heymind]

I'm working on a reverse proxy server via websocket and this great library is used as a multiplexer over single ws connection. It works fine when proxying small http request but will panic if body is large.

panic: runtime error: index out of range [29934] with length 7680

goroutine 13 [running]:
bufio.(*Reader).Read(0xc000443920, {0xc0005fa000, 0x1e00, 0xc00014bcc0})
        /usr/local/Cellar/go/1.17/libexec/src/bufio/bufio.go:218 +0x319
io.(*LimitedReader).Read(0xc00048e2a0, {0xc0005fa000, 0x153bb86, 0xc00014bd00})
        /usr/local/Cellar/go/1.17/libexec/src/io/io.go:473 +0x45
bytes.(*Buffer).ReadFrom(0xc0000a11d0, {0x18eb320, 0xc00048e2a0})
        /usr/local/Cellar/go/1.17/libexec/src/bytes/buffer.go:204 +0x98
io.copyBuffer({0x18ea9a0, 0xc0000a11d0}, {0x18eb320, 0xc00048e2a0}, {0x0, 0x0, 0x0})
        /usr/local/Cellar/go/1.17/libexec/src/io/io.go:409 +0x14b
io.Copy(...)
        /usr/local/Cellar/go/1.17/libexec/src/io/io.go:382
github.com/hashicorp/yamux.(*Stream).readData(0xc0000a20d0, {0xc00011c040, 0xc000000002, 0xc}, 0xc998, {0x18ea940, 0xc000443920})
        /Users/hey/go/pkg/mod/github.com/hashicorp/yamux@v0.0.0-20210826001029-26ff87cf9493/stream.go:482 +0x21a
github.com/hashicorp/yamux.(*Session).handleStreamMessage(0xc0000d9080, {0xc00011c040, 0xc00011c040, 0xc})
        /Users/hey/go/pkg/mod/github.com/hashicorp/yamux@v0.0.0-20210826001029-26ff87cf9493/session.go:550 +0x285
github.com/hashicorp/yamux.(*Session).recvLoop(0xc0000d9080)
        /Users/hey/go/pkg/mod/github.com/hashicorp/yamux@v0.0.0-20210826001029-26ff87cf9493/session.go:501 +0x114
github.com/hashicorp/yamux.(*Session).recv(0xc0005148a0)
        /Users/hey/go/pkg/mod/github.com/hashicorp/yamux@v0.0.0-20210826001029-26ff87cf9493/session.go:462 +0x1e
created by github.com/hashicorp/yamux.newSession
        /Users/hey/go/pkg/mod/github.com/hashicorp/yamux@v0.0.0-20210826001029-26ff87cf9493/session.go:113 +0x49b
exit status 2

https://github.com/hashicorp/yamux/blob/26ff87cf94932a054bffb8b8e926d94d44558b4d/stream.go#L471-L475

Maybe we should grow it's recv buffer large enough to avoid panics? I have tried this modification and it never panics again. But I don't know whether makes it right.

if s.recvBuf.Cap() < int(length) {
s.recvBuf.Grow(int(length) - s.recvBuf.Cap())
}

[dnephin]

Thank you for the bug report! We took a few minutes to look over the stack trace and the implementation of the readers and writers involved here.

From what we can tell, the issue does not seem to be the recvBuf capacity. It seems like the io.ReadWriteCloser that is being passed as the first conn argument to https://pkg.go.dev/github.com/hashicorp/yamux#Server may not correctly implement the io.Reader interface. Ref: https://pkg.go.dev/io#Reader

It seems like a read call to that reader is returning a number that is too large for "number of bytes read" (the n return value). This is causing the bufio.Reader to attempt to read past the capacity of the bytes.Buffer, which produces this panic. The relevant lines are here:

https://cs.opensource.google/go/go/+/refs/tags/go1.17.2:src/bufio/bufio.go;l=213-218

We expect that the reader should only return up to length bytes, because it is wrapped in an io.LimitedReader, but looking at https://cs.opensource.google/go/go/+/refs/tags/go1.17.2:src/io/io.go;l=461, we can see that all LimitedReader does is resize the slice, it don't force the underlying reader to limit its read, and it never validates the n that it gets back.

Is the conn io.ReadWriteCloser you pass in a custom implementation of io.ReadWriteCloser or from the stdlib? Does this explanation seem like it might explain the panic?

As far as we can tell at this point, it does not seem to be a bug in yamux.

[jefferai]

Closing due to lack of response.