panic in Association.handleCookieEcho

[zyxar]

Your environment.

• Version: v1.7.9
• Browser: N/A
• Other Information

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x61de04]
goroutine 11947982 [running]:
github.com/pion/sctp.(*Association).handleCookieEcho(0xc00013e340, 0xc00057e800, 0x0, 0x0, 0xc0021c45d0)
vendor/github.com/pion/sctp/association.go:993 +0x144
github.com/pion/sctp.(*Association).handleChunk(0xc00013e340, 0xc0021c4708, 0x102b600, 0xc00057e800, 0x0, 0x0)
vendor/github.com/pion/sctp/association.go:2055 +0x8f2
github.com/pion/sctp.(*Association).handleInbound(0xc00013e340, 0xc0004e4540, 0x30, 0x30, 0x30, 0x0)
vendor/github.com/pion/sctp/association.go:505 +0x395
github.com/pion/sctp.(*Association).readLoop(0xc00013e340)
vendor/github.com/pion/sctp/association.go:430 +0x227
created by github.com/pion/sctp.(*Association).init
vendor/github.com/pion/sctp/association.go:298 +0xa1

What did you do?

We had one crash report this afternoon. The stack is pasted above.
Current project is using webrtc/v2, which uses sctp v1.7.9. But I checked new changes and don't see any addressed the issue above.

The panic line is if !bytes.Equal(a.myCookie.cookie, c.cookie) { in below func:

// The caller should hold the lock.
func (a *Association) handleCookieEcho(c *chunkCookieEcho) []*packet {
	state := a.getState()
	a.log.Debugf("[%s] COOKIE-ECHO received in state '%s'", a.name, getAssociationStateString(state))
	switch state {
	default:
		return nil
	case established:
		if !bytes.Equal(a.myCookie.cookie, c.cookie) {
			return nil
		}
	case closed, cookieWait, cookieEchoed:
		if !bytes.Equal(a.myCookie.cookie, c.cookie) {
			return nil
		}

since a *Association is not nil, so I guess either a.myCookie or c is nil.

c comes from:

func (a *Association) handleInbound(raw []byte) error {
	p := &packet{}
	if err := p.unmarshal(raw); err != nil {
		a.log.Warnf("[%s] unable to parse SCTP packet %s", a.name, err)
		return nil
	}

	if err := checkPacket(p); err != nil {
		a.log.Warnf("[%s] failed validating packet %s", a.name, err)
		return nil
	}

	a.handleChunkStart(p)

	for _, c := range p.chunks {
		if err := a.handleChunk(p, c); err != nil {
			return err
		}
	}

and the unmarshalling looks good:

		var c chunk
		switch chunkType(raw[offset]) {
		case ctInit:
			c = &chunkInit{}
		case ctInitAck:
			c = &chunkInitAck{}
		case ctAbort:
			c = &chunkAbort{}
		case ctCookieEcho:
			c = &chunkCookieEcho{}
		case ctCookieAck:
			c = &chunkCookieAck{}
		case ctHeartbeat:
			c = &chunkHeartbeat{}
		case ctPayloadData:
			c = &chunkPayloadData{}
		case ctSack:
			c = &chunkSelectiveAck{}
		case ctReconfig:
			c = &chunkReconfig{}
		case ctForwardTSN:
			c = &chunkForwardTSN{}
		case ctError:
			c = &chunkError{}
		default:
			return errors.Errorf("Failed to unmarshal, contains unknown chunk type %s", chunkType(raw[offset]).String())
		}

as c is a chunk (interface) to a concrete type (pointer). seems no chance to panic on dereferencing c.cookie. So probably a.myCookie is nil in this case?

myCookie is initialised in handleInit:

	if a.myCookie == nil {
		var err error
		if a.myCookie, err = newRandomStateCookie(); err != nil {
			return nil, err
		}
	}

but in any case it failed to init myCookie, the error would be ignored in caller function handleChunk. However I'm not sure why rand.Reader would fail in this case:

func newRandomStateCookie() (*paramStateCookie, error) {
	randCookie := make([]byte, 32)
	_, err := rand.Read(randCookie)
	// crypto/rand.Read returns n == len(b) if and only if err == nil.
	if err != nil {
		return nil, err
	}

	s := &paramStateCookie{
		cookie: randCookie,
	}

	return s, nil
}

or handleCookieEcho happened before handleInit?

What did you expect?

No panic.

[at-wat]

I could reproduce the panic by adding a unit test. Will create a PR.