From c914451e2da68d0aefd0641c217da169b1dfaa94 Mon Sep 17 00:00:00 2001 From: nlarsen Date: Sat, 28 Nov 2020 17:04:15 +0100 Subject: [PATCH 1/2] preparation: swap 'if's leaving logic 100% as before --- src/SparkFun_Ublox_Arduino_Library.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/SparkFun_Ublox_Arduino_Library.cpp b/src/SparkFun_Ublox_Arduino_Library.cpp index 81994a1..4a263ba 100644 --- a/src/SparkFun_Ublox_Arduino_Library.cpp +++ b/src/SparkFun_Ublox_Arduino_Library.cpp @@ -924,14 +924,14 @@ void SFE_UBLOX_GPS::processUBX(uint8_t incoming, ubxPacket *incomingUBX, uint8_t uint16_t startingSpot = incomingUBX->startingSpot; if (incomingUBX->cls == UBX_CLASS_NAV && incomingUBX->id == UBX_NAV_PVT) startingSpot = 0; - //Begin recording if counter goes past startingSpot - if ((incomingUBX->counter - 4) >= startingSpot) + // Check if this is payload data which should be ignored + if (ignoreThisPayload == false) { - //Check to see if we have room for this byte - if (((incomingUBX->counter - 4) - startingSpot) < MAX_PAYLOAD_SIZE) //If counter = 208, starting spot = 200, we're good to record. + //Begin recording if counter goes past startingSpot + if ((incomingUBX->counter - 4) >= startingSpot) { - // Check if this is payload data which should be ignored - if (ignoreThisPayload == false) + //Check to see if we have room for this byte + if (((incomingUBX->counter - 4) - startingSpot) < MAX_PAYLOAD_SIZE) //If counter = 208, starting spot = 200, we're good to record. { incomingUBX->payload[incomingUBX->counter - 4 - startingSpot] = incoming; //Store this byte into payload array } From 386e0de91e8c80f4cb65e262a7de50313edc73a2 Mon Sep 17 00:00:00 2001 From: nlarsen Date: Sat, 28 Nov 2020 17:30:28 +0100 Subject: [PATCH 2/2] fix critical array overrun bug (incomingUBX->payload[] index was only checked against MAX_PAYLOAD_SIZE, but incomingUBX can also be packetAck or packetBuf which have much smaller buffers of only 2 bytes) --- src/SparkFun_Ublox_Arduino_Library.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/SparkFun_Ublox_Arduino_Library.cpp b/src/SparkFun_Ublox_Arduino_Library.cpp index 4a263ba..3c31247 100644 --- a/src/SparkFun_Ublox_Arduino_Library.cpp +++ b/src/SparkFun_Ublox_Arduino_Library.cpp @@ -760,6 +760,9 @@ void SFE_UBLOX_GPS::processRTCM(uint8_t incoming) //a subset of bytes within a larger packet. void SFE_UBLOX_GPS::processUBX(uint8_t incoming, ubxPacket *incomingUBX, uint8_t requestedClass, uint8_t requestedID) { + size_t max_payload_size = (activePacketBuffer == SFE_UBLOX_PACKET_PACKETCFG) ? MAX_PAYLOAD_SIZE : 2; + bool overrun = false; + //Add all incoming bytes to the rolling checksum //Stop at len+4 as this is the checksum bytes to that should not be added to the rolling checksum if (incomingUBX->counter < incomingUBX->len + 4) @@ -931,10 +934,14 @@ void SFE_UBLOX_GPS::processUBX(uint8_t incoming, ubxPacket *incomingUBX, uint8_t if ((incomingUBX->counter - 4) >= startingSpot) { //Check to see if we have room for this byte - if (((incomingUBX->counter - 4) - startingSpot) < MAX_PAYLOAD_SIZE) //If counter = 208, starting spot = 200, we're good to record. + if (((incomingUBX->counter - 4) - startingSpot) < max_payload_size) //If counter = 208, starting spot = 200, we're good to record. { incomingUBX->payload[incomingUBX->counter - 4 - startingSpot] = incoming; //Store this byte into payload array } + else + { + overrun = true; + } } } } @@ -942,7 +949,7 @@ void SFE_UBLOX_GPS::processUBX(uint8_t incoming, ubxPacket *incomingUBX, uint8_t //Increment the counter incomingUBX->counter++; - if (incomingUBX->counter == MAX_PAYLOAD_SIZE) + if (overrun or incomingUBX->counter == MAX_PAYLOAD_SIZE) { //Something has gone very wrong currentSentence = NONE; //Reset the sentence to being looking for a new start char