Payload buffer overrun fix

[nelarsen]

There is a latent buffer overrun issue in SFE_UBLOX_GPS::processUBX()

Bug
in processUBX(), when writing to incomingUBX->payload[], the index was only checked against MAX_PAYLOAD_SIZE (256). But incomingUBX can also be packetAck or packetBuf which have much smaller buffers (2). In case of certain corrupted input, a buffer overrun is not ruled out and happened in our setup. This code change fixed the problem for us and I believe there should be no side effects.

Occurance
The bug will only show up if the input stream from the GPS is corrupted. Here is a real example of a such corrupted stream.

I believe that the library should be able to handle corrupted streams and should never allow a buffer to overflow. The pull request rules out a buffer overrun.

Real example of corrupted stream
as processed by process(), the decimal number is the value of ubxFrameCounter at the beginning of process()
[025] 00
[026] ae
[027] 72

[028] b5
[001] 62
[002] 05 <--- UBX-ACK class 0x05 (requestedClass differs and is 0x06)
[003] 01 <--- UBX-ACK-ACK id 0x01 (requestedId differs and is 0x00)
<--- Unexpected end of packet (something is missing here)
[004] 82 <--- interpreted as length, is probably last checksum byte of a mostly lost packet
[005] b5 <--- begin of next packet, still being interpreted as length (b5 82 -> len=46466)
[006] 62
[007] 05
[008] 01

[PaulZC]

Hi Nils (@nelarsen ),
Thank you for submitting this PR.
I do like the changes you have made - but I will need to test them before adding them to the library. I will need to make sure that the code still works correctly when (e.g.) we have autoPVT messages arriving out of sequence.
Can you please do one thing for me? Can you please re-target this PR at the release_candidate branch, so I can test your changes before merging them into the main branch? I have updated CONTRIBUTING.md - but the changes are still in the release_candidate branch!
https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/release_candidate/CONTRIBUTING.md
I will be releasing a new version of the library in a day or two and it would be good to include your changes, so please do re-target if you have time.
Thanks and best wishes,
Paul

[nelarsen]

Hallo Paul,
thanks for following up on this! It is my first pull request, sorry for not knowing the customs. I have re-targeted the PR now.
I do not understand the 'autoPVT out of sequence', so please test thoroughly (-;
If you finally choose to merge my changes, there will still be some responsibility left to the user of the library: Any ubxPacket provided from outside must have an allocated buffer of MAX_PAYLOAD_SIZE.
Best regards, Nils

[PaulZC]

Hi Nils,
Perfect - thank you.
If you want to try "autoPVT", you can find a good example for the ZED-F9P here:
https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/tree/release_candidate/examples/ZED-F9P/Example13_autoHPPOSLLH
The example uses both PVT and HPPOSLLH messages. If you select this option then you can check that a PVT message arriving "automatically" does not cause errors for the HPPOSLLH which is polled normally.
Best wishes,
Paul

[PaulZC]

Hi Nils (@nelarsen),
Thanks again for contributing to the library. Can we please contact you via email? We are working on Version 2.0 of the u-blox library and we would appreciate your thoughts and comments. You can find my email address on my home page:
https://github.com/PaulZC
Very best wishes,
Paul (& Nathan @nseidle)