lightaprs / LightAPRS-W-1.0

Arduino based APRS & WSPR Tracker
GNU General Public License v3.0
62 stars 15 forks source link

aprs telemetry fix #5

Closed dl9sau closed 2 years ago

dl9sau commented 2 years ago

After some time running, still at 30m above sea, not moving, I got strange reports about speed and height. During development, this occured sometimes when assigning variables of incorrect types without cast, which smashed the stack. This is a problem of the architecture. -> If you see inplausible values, that may due to other bugs in own code enhancements ;). But today, I saw strange values again. I looked deeper and found:

void updateTelemetry() { sprintf(telemetry_buff, "%03d", gps.course.isValid() ? (int)gps.course.deg() : 0); telemetry_buff[3] += '/'; sprintf(telemetry_buff + 4, "%03d", gps.speed.isValid() ? (int)gps.speed.knots() : 0); telemetry_buff[7] = '/'; telemetry_buff[8] = 'A'; telemetry_buff[9] = '=';

If you look at the second line, you see a wrong assignment: '+='. It shouold be '='.

You have luck. The sprintf function above write "nnn\0", thus posistion 3 is alsways 0. and 0 += '/' is '/'. Imagine gps.course.deg() returns -109 or > 1009 (that should never happen), in that position is the number 9 (ascii 57). then our result would be 57+47 ('/') = 104. Telemetry buf is size of 100 -> buffer overflow.

So telemetry_buff[3] = '/'; is the correct fix.

This fix should also applied to the projects https://github.com/lightaprs/LightAPRS-1.0 LightAPRS-{hab,pico-balloon,vehicle}

lightaprs commented 2 years ago

Thanks for the fix :)