[bug]

[secsys-go]

Describe the bug
We used the Fuzz engine to modify some Test(TestJoinMessages) data, and then the following crash appeared. We hope to get help from developers to confirm whether the bug makes sense in a real-world scenario

To Reproduce

func TestJoinMessages(t *testing.T) {
	var1 := "\x00"
	var2 := "\x00\x00"
	var3 := "\x00\x00\x00"
	var4 := "0000"
	var5 := "00\b\x00\x00"
	var6 := "\x00"
	var7 := "\x00\x00"
	var8 := "\x00\x000"
	var9 := "0000"
	messages := []string{var1, var2, var3, var4, var5, var6, var7, var8, var9}

	var10 := 2096
	var11 := 590024534710562816
	var12 := 3458764513820540928
	var13 := 9003059916848
	var14 := 3472328089260785664
	var15 := 137375792
	var16 := 3472328296224522240

	var17 := ""
	var18 := "0"

	for _, readChunk := range []int{var10, var11, var12, var13, var14, var15, var16} {
		for _, term := range []string{var17, var18} {
			var connBuf bytes.Buffer
			wc := newTestConn(nil, &connBuf, true)
			rc := newTestConn(&connBuf, nil, false)
			var19 := []int{8, 2304783338713136, 3472275312702652416, 35168202800, 3472328295419215872, 536624, 3472328296227667968, 8, 2304783338713136}
			for idx, m := range messages {
				tmpVar := var19[idx]
				wc.WriteMessage(tmpVar, []byte(m))
			}

			var result bytes.Buffer
			_, err := io.CopyBuffer(&result, JoinMessages(rc, term), make([]byte, readChunk))
			if IsUnexpectedCloseError(err, CloseAbnormalClosure) {
				t.Errorf("readChunk=%d, term=%q: unexpected error %v", readChunk, term, err)
			}
			want := strings.Join(messages, term) + term
			if result.String() != want {
				t.Errorf("readChunk=%d, term=%q, got %q, want %q", readChunk, term, result.String(), want)
			}
		}
	}
}

Crash log

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x6d2ecb]

goroutine 19 [running]:
testing.tRunner.func1.2({0x7091e0, 0x9d4300})
	/home/zjx/.local/go/src/testing/testing.go:1211 +0x24e
testing.tRunner.func1()
	/home/zjx/.local/go/src/testing/testing.go:1214 +0x218
panic({0x7091e0, 0x9d4300})
	/home/zjx/.local/go/src/runtime/panic.go:1038 +0x215
github.com/gorilla/websocket.(*fakeNetConn).Write(0xc00015e160, {0xc00019e000, 0x9e3860, 0x0})
	<autogenerated>:1 +0x2b
github.com/gorilla/websocket.(*Conn).WriteControl(0xc000176f20, 0x8, {0xc0000e0970, 0x0, 0x40b8f5}, {0x1, 0x3fe, 0x9e3860})
	/home/zjx/workspace/gowork/src/purelib/crashConfirm/websocket/conn.go:464 +0x4b6
github.com/gorilla/websocket.(*Conn).SetCloseHandler.func1(0xc000176f20, {0x4150a5, 0x7f79bccd72b8})
	/home/zjx/workspace/gowork/src/purelib/crashConfirm/websocket/conn.go:1139 +0xa5
github.com/gorilla/websocket.(*Conn).advanceFrame(0xc000176f20)
	/home/zjx/workspace/gowork/src/purelib/crashConfirm/websocket/conn.go:970 +0xb03
github.com/gorilla/websocket.(*Conn).NextReader(0xc000176f20)
	/home/zjx/workspace/gowork/src/purelib/crashConfirm/websocket/conn.go:1009 +0xc5
github.com/gorilla/websocket.(*joinReader).Read(0xc000125290, {0xc00019c000, 0x200, 0x200})
	/home/zjx/workspace/gowork/src/purelib/crashConfirm/websocket/join.go:28 +0x56
bytes.(*Buffer).ReadFrom(0xc000125260, {0x7cc460, 0xc000125290})
	/home/zjx/.local/go/src/bytes/buffer.go:204 +0x98
io.copyBuffer({0x7cc1c0, 0xc000125260}, {0x7cc460, 0xc000125290}, {0xc000154900, 0x830, 0x830})
	/home/zjx/.local/go/src/io/io.go:409 +0x14b
io.CopyBuffer({0x7cc1c0, 0xc000125260}, {0x7cc460, 0xc000125290}, {0xc000154900, 0x92cb00, 0x995c60})
	/home/zjx/.local/go/src/io/io.go:396 +0x3c
github.com/gorilla/websocket.TestJoinMessages(0xc00010bd40)
	/home/zjx/workspace/gowork/src/purelib/crashConfirm/websocket/join_test.go:73 +0x525

Desktop:

• OS: Linux r920 4.15.0-159-generic #167-Ubuntu

iris.Version

• e.g. commit af47554 (HEAD -> master, origin/master, origin/HEAD)

[garyburd]

It is not a real world scenario.

Here's what's happening:

• The modified test writes a close message to connBuf. The original test writes binary messages only.
• The connection rc calls WriteControl to echo the close message to the peer.
• Write on the underlying fake network connection panics because the fake connection does not have a writer.

Here's a simple program that reproduces the panic:

var connBuf bytes.Buffer
wc := newTestConn(nil, &connBuf, true)
rc := newTestConn(&connBuf, nil, false)
wc.WriteMessage(CloseMessage, []byte{})
var result bytes.Buffer
io.CopyBuffer(&result, JoinMessages(rc, ""), make([]byte, 100))

Here's a fix for the panic:

var connBuf bytes.Buffer
var connBuf2 bytes.Buffer // <-- writer for fake network connection
wc := newTestConn(nil, &connBuf, true)
rc := newTestConn(&connBuf, &connBuf2, false)
wc.WriteMessage(CloseMessage, []byte{})
var result bytes.Buffer
io.CopyBuffer(&result, JoinMessages(rc, ""), make([]byte, 100))

Because JoinMessage ends the stream on any error from the websocket connection, there's no value in extending the test to handle close messages.