panic: runtime error: slice bounds out of range [:3524] with capacity 1500

[jmattheis]

Your environment.

Version: v2.0.5 Edge Version 88.0.705.74 (64-Bit) Chrome Version 88.0.4324.182 (x86_64) Mac 10.15.7 (19H524)

panic: runtime error: slice bounds out of range [:3524] with capacity 1500

goroutine 60 [running]:
github.com/pion/turn/v2.(*Server).readLoop(0xc000271a40, 0xd85b20, 0xc0001a10e0, 0xc00006afc0)
	/home/runner/go/pkg/mod/github.com/pion/turn/[email protected]/server.go:150 +0x3ac
created by github.com/pion/turn/v2.NewServer.func2
	/home/runner/go/pkg/mod/github.com/pion/turn/[email protected]/server.go:102 +0x1b2

Originally reported in https://github.com/screego/server/issues/74

What did you do?

Used the turn server (:.

---

When n > s.inboundMTU a debug logging is written, I think it should return or modify n that buf[:n] doesn't panic

https://github.com/pion/turn/blob/73f74b13d23ac0d9dcce31610d78d60a99a34fe6/server.go#L147-L162

[Sean-Der]

oof, yes! I am thinking the right thing to do here is return.

We can't read again right? I never actually reproduced this myself, sorry I should have put more effort into this one :/

[jmattheis]

Maybe readFrom shouldn't return a number higher than the buffer. Similar to the standard golang apis. But I haven't tried yet to understand why this is the case.

[jech]

I believe this was fixed in 13867664acbcf7a2b55f561fc4ed61b46638438d.

However, I agree with @jmattheis that ReadFrom shouldn't be returning a value larger than the packet, since this makes it inconsistent with PacketConn.ReadFrom, which might lead to confusion further on.

[jerry-tao]

I think the problem is stun_conn.go#L68, if the frame size is larger than MTU, it will panic. I think change it to:

	if errors.Is(err, errInvalidTURNFrame) {
		return 0, nil, err
	} else if err == nil {
		if n>len(p){
			return 0,nil,errMTUTooLarge
		}
		copy(p, s.buff[:n])
		s.buff = s.buff[n:]

		return n, s.nextConn.RemoteAddr(), nil
	}

could fix the problem, but i just start to learn about turn, if this is ok i will submit a PR @jech @Sean-Der .

[stv0g]

I agree with @jerry-tao here. However, does not this also apply to TCP?

I find the naming of InboundMTU misleading. I think we need to in fact refer to it as the maximum TURN/STUN message length which could be larger than the MTU in case we are using TURN over TCP/TLS?

I quickly checked the RFCs to find if they specify the maximum STUN message length somewhere. But I couldnt find it..

[loreskovic]

This is still an issue in v2.0.9. I've had it happen as part of a Livekit server with TURN/TLS and TURN/UDP enabled. I don't know how to reproduce it, it just happens occasionally. Here is my trace:

panic: runtime error: slice bounds out of range [:21540] with capacity 1600

goroutine 2540914 [running]:
github.com/pion/turn/v2.(*Server).readLoop(0xc0016f4000, {0x128fd50, 0xc001804ea0}, 0xc000130d20)
	/go/pkg/mod/github.com/pion/turn/[email protected]/server.go:176 +0x39b
created by github.com/pion/turn/v2.NewServer.func2
	/go/pkg/mod/github.com/pion/turn/[email protected]/server.go:114 +0x397

I can provide more information if necessary.

[jmattheis]

Looks like this issue may have been fixed with https://github.com/pion/turn/commit/d87d0f3f81e0ea04a13455a193ceca4ad1fb060c which was released in the latest version.

[stv0g]

Thanks :)