muka / go-bluetooth

Golang bluetooth client based on bluez DBus interfaces
Apache License 2.0
653 stars 124 forks source link

ParseIBeacon should check frames length #114

Closed fermuch closed 4 years ago

fermuch commented 4 years ago

I'm getting the following error:

panic: runtime error: slice bounds out of range [:18] with capacity 7

goroutine 178 [running]:
github.com/muka/go-bluetooth/api/beacon.(*Beacon).ParseIBeacon(0xc00052c0b0, 0xc0004e6610, 0x7, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/fermuch/go/pkg/mod/github.com/muka/go-bluetooth@v0.0.0-20200803190124-3e6e2c681b72/api/beacon/beacon_ibeacon.go:41 +0x293
github.com/muka/go-bluetooth/api/beacon.(*Beacon).parserIBeacon(0xc00052c0b0, 0xc000606030, 0x0)
        /home/fermuch/go/pkg/mod/github.com/muka/go-bluetooth@v0.0.0-20200803190124-3e6e2c681b72/api/beacon/beacon.go:143 +0x113
github.com/muka/go-bluetooth/api/beacon.(*Beacon).Parse(0xc00052c0b0, 0xc0001d6f48)
        /home/fermuch/go/pkg/mod/github.com/muka/go-bluetooth@v0.0.0-20200803190124-3e6e2c681b72/api/beacon/beacon.go:117 +0x139
github.com/muka/go-bluetooth/api/beacon.(*Beacon).WatchDeviceChanges.func1(0xc000026480, 0x6c2780, 0xc00001c120, 0xc0000264e0, 0xc00052c0b0)
        /home/fermuch/go/pkg/mod/github.com/muka/go-bluetooth@v0.0.0-20200803190124-3e6e2c681b72/api/beacon/beacon.go:69 +0x13e
created by github.com/muka/go-bluetooth/api/beacon.(*Beacon).WatchDeviceChanges
        /home/fermuch/go/pkg/mod/github.com/muka/go-bluetooth@v0.0.0-20200803190124-3e6e2c681b72/api/beacon/beacon.go:58 +0xe3
exit status 2

ParseIBeacon should check if the frames length makes sense before trying to read it, if I understand things correctly.

muka commented 4 years ago

Thanks, you are right, it misses any basic input validation. Can you provide a PR to fix this?

fermuch commented 4 years ago

Done!

I started Go for the first time yesterday, so please do review with care before merging. I think the changes are small and should not interfere with anything else.

muka commented 4 years ago

Great work! Will review and merge as soon as possible, thank you