Skip to content

Conversation

mcoops
Copy link
Contributor

@mcoops mcoops commented Feb 3, 2024

On rare occasions, it is possible that the length field in the RadioTap layer is bigger than the actual data length which results in a panic when trying to reference the payload. If it's bigger, simply truncate the RadioTap length to the length of the actual data to allow processing of what we can safely.

On rare occasions, it is possible that the length field in the RadioTap
layer is bigger than the actual data length which results in a panic
when trying to reference the payload. If it's bigger, simply truncate
the RadioTap length to the length of the actual data to allow processing
of what we can safely.
@mosajjal
Copy link
Contributor

mosajjal commented Feb 6, 2024

hi. can you please attach a pcap file that causes this issue.

@mcoops
Copy link
Contributor Author

mcoops commented Feb 7, 2024

Certainly! We're you after just a pcap which creates the panic? Do you want that committed somewhere? Or were you more after realistic traffic - although that's going to be a lot harder to find/create/sanitize.

@mosajjal
Copy link
Contributor

mosajjal commented Feb 7, 2024

I'm looking to build a test case that fails before this PR and is successful after this PR. at least to know what the corner case actually is :)

@mcoops
Copy link
Contributor Author

mcoops commented Feb 13, 2024

Hrmm I have no idea how you actually upload a pcap to GH but we can just use the unit test I added and make a pcap with Python+scapy:

from scapy.all import *
import codecs

pkt = RadioTap(codecs.decode('0000ff002e48000010026c09a000c6070000d4000000881fa1ae9dcbc6304b4b', 'hex'))
wrpcap('panic.pcap', pkt)

Then manually testing with something like:

func main() {
	filePath := flag.String("file", "", "Path to pcap file")
	flag.Parse()

	if *filePath == "" {
		fmt.Println("Provide the path to the pcap file using -file flag")
		os.Exit(1)
	}

	handle, err := pcap.OpenOffline(*filePath)
	if err != nil {
		log.Fatal(err)
	}
	defer handle.Close()

	data, _, err := handle.ZeroCopyReadPacketData()

	if err != nil {
		fmt.Println(err)
	}

	var radioTap layers.RadioTap
	if err := radioTap.DecodeFromBytes(data, gopacket.NilDecodeFeedback); err != nil {
		fmt.Println(err)
	}
	fmt.Println(radioTap.Length)
}

That at least shows the corner case as I set the RadioTap.Length to 255:

$ go run main.go -file ~/panic.pcap
panic: runtime error: slice bounds out of range [255:32]

goroutine 1 [running]:
github.com/gopacket/gopacket/layers.(*RadioTap).DecodeFromBytes(0x38772788?, {0x1f89e50?, 0x6dbd80?, 0x20?}, {0x5ecae8?, 0xc33b20?})
        /home/user/go/pkg/mod/github.com/gopacket/gopacket@v1.2.0/layers/radiotap.go:868 +0xbbc
main.main()
        /home/user/tmp/main.go:37 +0x257
exit status 2

Then obviously with the patch it just runs and shows the Length has been truncated to the frame size:

$ go run main.go -file ~/panic.pcap 
32

@mosajjal mosajjal merged commit 6e77ffa into gopacket:master Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants