panic in the production server

[shengery]

Greetings,

We are deploying our chat servers in the production env, the connsrv is the gateway server between clients and internal logic server, which is serving websocket requests , however , we met an unusual panic last day The panic stack is :

panic: runtime error: makeslice: len out of range

goroutine 5626477 [running]:
aladinfun.com/midware/goim/vendor/github.com/gobwas/ws.ReadFrame(0x7f5be12140c0, 0xc42000ece0, 0x60700, 0x2424ebb535b8385d, 0xfba2344b01, 0x0, 0x0, 0x0, 0x0, 0x0)
 /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/vendor/github.com/gobwas/ws/read.go:115 +0xd7
aladinfun.com/midware/goim/srv/connsrv.wsReceive(0xe4a5e0, 0xc42000ece0, 0x8, 0x9d4440, 0xc422d8c730, 0xc4231811e0, 0x18)
 /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/srv/connsrv/conn_ws.go:49 +0xa6
aladinfun.com/midware/goim/srv/connsrv.clientLoop(0xe4a5e0, 0xc42000ece0, 0xc4203e2f80)
 /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/srv/connsrv/conn_handler.go:49 +0x150
aladinfun.com/midware/goim/srv/connsrv.uriRoute(0xe4a5e0, 0xc42000ece0, 0xc4203e2f80)
 /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/srv/connsrv/conn_listener.go:85 +0x133
aladinfun.com/midware/goim/srv/connsrv.handleConn(0xe4a5e0, 0xc42000ece0)
 /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/srv/connsrv/conn_listener.go:69 +0x25f
created by aladinfun.com/midware/goim/srv/connsrv.listenTCP
 /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/srv/connsrv/conn_listener.go:57 +0x4fd

The location of the panic in read.go is :

func ReadFrame(r io.Reader) (f Frame, err error) {
    f.Header, err = ReadHeader(r)
    if err != nil {
        return
    }

    if f.Header.Length > 0 {
        // int(f.Header.Length) is safe here cause we have
        // checked it for overflow above in ReadHeader.
                //  panic here!! 
        f.Payload = make([]byte, int(f.Header.Length))
        _, err = io.ReadFull(r, f.Payload)
    }

    return
}

However , we don't know how much the frame.length is..

The /data/home/user_00/go_workspace/src/aladinfun.com/midware/goim/srv/connsrv/conn_ws.go:49 is below

45 func wsReceive(conn net.Conn) (data []byte, err error) {
46  if config.SrvCfg.ClientTimeoutR != 0 {
47      conn.SetReadDeadline(time.Now().Add(time.Duration(config.SrvCfg.ClientTimeoutR) * time.Millisecond))
48  }
49  frm, err := ws.ReadFrame(conn)
50  if err != nil {
51      xlog.LevelLogfn(logger.DebugLog, "ws ReadFrame failed, %s", err.Error())
52      return
53  }
54  if frm.Header.Masked {
55      ws.Cipher(frm.Payload, frm.Header.Mask, 0)
56  }
57
58  data = frm.Payload
59  // if data, _, err = crypto.Base64Decode(frm.Payload); err != nil {
60  //  xlog.LevelLogfn(logger.DebugLog, "base64 decode failed, %s", err.Error())
61  //  return
62  // }
63  xlog.LevelLogfn(logger.TraceLog, "receive buffer: %v", data)
64  return
65 }

[gobwas]

Hi @shengery!

Looks like the f.Header.Length is too big:

// /usr/local/go/src/runtime/slice.go

func makeslice(et *_type, len, cap int) slice {
    // NOTE: The len > maxElements check here is not strictly necessary,
    // but it produces a 'len out of range' error instead of a 'cap out of range' error
    // when someone does make([]T, bignumber). 'cap out of range' is true too,
    // but since the cap is only being supplied implicitly, saying len is clearer.
    // See issue 4085.
    maxElements := maxSliceCap(et.size)
    if len < 0 || uintptr(len) > maxElements {
        panic(errorString("makeslice: len out of range"))
    }
    ...
}

Looking at ws.ReadHeader code it is not possible to receive negative frame length. So, there could be few cases when this happens:

• when connection streams some trash bytes that are not actually websocket frames
• when someone sends huge frames to get the ddos behaviour of your service

So the solution is to split ws.ReadFrame into ws.ReadHeader, check the header, and then ws.ReadFrame.

Also, take a look at this helper function to preare RFC header checks. But notice that there is no Length check because it is not limited by RFC.

[shengery]

Do you mean that I handle the frame.Header in my code instead of directly calling ws.ReadFrame?

[gobwas]

Yes, ReadFrame() just makes ReadHeader() and then make([]byte, header.Length).

[shengery]

OK,thanks for your reply, I will call ReadHeader and readbody seperately