search

wmnsk / go-pfcp

PFCP (Packet Forwarding Control Protocol) in pure Go.
MIT License
125 stars 56 forks source link

Don't panic on truncated input to `IE.UnmarshalBinary` #90

Closed pudelkoM closed 2 years ago

pudelkoM commented 2 years ago

This PR adds an additional length check comparing the actual buffer length to the parsed IE.Length field before re-slicing the buffer as Payload.

We encountered the following panic trace in our setup when packets either get corrupted/malformed/truncated. While those packets should not be successfully parsed, go-pfcp should also not just crash.

I also added a rudimentary test to demonstrate the issue, it can surely be restructured to fit the existing code.

panic: runtime error: slice bounds out of range [:50] with capacity 41
goroutine 13 [running]:
github.com/wmnsk/go-pfcp/ie.(*IE).UnmarshalBinary(0x300c00003fa00, {0xc0002eebb3, 0x2, 0x3})
    /pfcpiface/vendor/github.com/wmnsk/go-pfcp/ie/ie.go:367 +0x187
github.com/wmnsk/go-pfcp/ie.Parse({0xc0002eebb3, 0x29, 0x29}) 
    /pfcpiface/vendor/github.com/wmnsk/go-pfcp/ie/ie.go:339 +0x48
github.com/wmnsk/go-pfcp/ie.ParseMultiIEs({0xc0002ee610, 0x5dc, 0x5dc}) 
    /pfcpiface/vendor/github.com/wmnsk/go-pfcp/ie/ie.go:628 +0x8c 
github.com/wmnsk/go-pfcp/message.(*SessionEstablishmentRequest).UnmarshalBinary(0xc000232000, {0xc0002ee600, 0x51a530, 0xc000298008}) 
    /pfcpiface/vendor/github.com/wmnsk/go-pfcp/message/session-establishment-request.go:291 +0x6e
github.com/wmnsk/go-pfcp/message.Parse({0xc0002ee600, 0x5dc, 0x5dc}) 
    /pfcpiface/vendor/github.com/wmnsk/go-pfcp/message/message.go:116 +0x3ab
main.pfcpifaceMainLoop(0xc00037e500, {0xc0000ce240, 0x0}, {0xc0000ce250, 0xf}, {0xc0000ce260, 0x7}, {0x0, 0x0})
    /pfcpiface/conn.go:133 +0x8e5
created by main.main
    /pfcpiface/main.go:231 +0x94f

More details: https://github.com/omec-project/upf-epc/pull/366

krsna1729 commented 2 years ago

@wmnsk will there be a new a tag?

wmnsk commented 2 years ago

@krsna1729 Done!