Skip to content

Commit

Permalink
[FIXED] Websocket: possible panic when decoding CLOSE frame
Browse files Browse the repository at this point in the history
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
  • Loading branch information
kozlovic committed Sep 12, 2021
1 parent eb89c8a commit 556baf8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
19 changes: 10 additions & 9 deletions ws.go
Expand Up @@ -372,7 +372,6 @@ func (r *websocketReader) handleControlFrame(frameType wsOpCode, buf []byte, pos
var payload []byte
var err error

statusPos := pos
if rem > 0 {
payload, pos, err = wsGet(r.r, buf, pos, rem)
if err != nil {
Expand All @@ -382,17 +381,19 @@ func (r *websocketReader) handleControlFrame(frameType wsOpCode, buf []byte, pos
switch frameType {
case wsCloseMessage:
status := wsCloseStatusNoStatusReceived
body := ""
body := _EMPTY_
// If there is a payload, it should contain 2 unsigned bytes
// that represent the status code and then optional payload.
if len(payload) >= 2 {
status = int(binary.BigEndian.Uint16(buf[statusPos : statusPos+2]))
body = string(buf[statusPos+2 : statusPos+len(payload)])
if body != "" && !utf8.ValidString(body) {
// https://tools.ietf.org/html/rfc6455#section-5.5.1
// If body is present, it must be a valid utf8
status = wsCloseStatusInvalidPayloadData
body = "invalid utf8 body in close frame"
status = int(binary.BigEndian.Uint16(payload[:2]))
if len(payload) > 2 {

This comment has been minimized.

Copy link
@derekcollison

derekcollison Sep 13, 2021

Member

Looks strange, had to re-read several times to catch it. Maybe a better way to represent?

body = string(payload[2:])
if body != _EMPTY_ && !utf8.ValidString(body) {
// https://tools.ietf.org/html/rfc6455#section-5.5.1
// If body is present, it must be a valid utf8
status = wsCloseStatusInvalidPayloadData
body = "invalid utf8 body in close frame"
}
}
}
r.nc.wsEnqueueCloseMsg(status, body)
Expand Down
23 changes: 23 additions & 0 deletions ws_test.go
Expand Up @@ -226,6 +226,29 @@ func TestWSParseControlFrames(t *testing.T) {
if err != io.EOF || n != 0 {
t.Fatalf("Error on read: n=%v err=%v", n, err)
}

// Write a CLOSE without payload
mr.buf.Write([]byte{136, 2, 3, 232})
n, err = r.Read(p)
if err != io.EOF || n != 0 {
t.Fatalf("Error on read: n=%v err=%v", n, err)
}

// Write a CLOSE with invalid status
mr.buf.Write([]byte{136, 1, 100})
n, err = r.Read(p)
if err != io.EOF || n != 0 {
t.Fatalf("Error on read: n=%v err=%v", n, err)
}

// Write CLOSE with valid status and payload but call with a read buffer
// that has capacity of 1.
mr.buf.Write([]byte{136, 6, 3, 232, 't', 'e', 's', 't'})
pl := []byte{136}
n, err = r.Read(pl[:])
if err != io.EOF || n != 0 {
t.Fatalf("Error on read: n=%v err=%v", n, err)
}
}

func TestWSParseInvalidFrames(t *testing.T) {
Expand Down

0 comments on commit 556baf8

Please sign in to comment.