testing review system #1

GoogleCodeExporter · 2015-07-31T23:05:55Z

Purpose of code changes on this branch:


When reviewing my code changes, please focus on:


After the review, I'll merge this branch into:
/trunk

Original issue reported on code.google.com by [email protected] on 24 Sep 2013 at 3:00

Attachments:

string_scribble.patch

The text was updated successfully, but these errors were encountered:

Previously, a new track was created when the event "GPS Logger turned on" was read. This is correct for the MTK_LOGGER only, but not for the Holux devices M241 & GR245. Case GPSBabel#1: Firmware versions where the event is never logged. Effect: One large track is created. Applies to GR245 v1.x Case GPSBabel#2: Firmware versions where the event is logged, but not at the beginning of a track, but _after_ the first track point of a track (at least most of the times). Applies to GR245 v2.00, M241 v1.12, v1.13 For all of the currently known Holux firmware versions, the correct indication of a track start is the "Log period change" / "Log distance change" event. This commit adds some more testcases (241 case provided by Erik Krause) that were suffering from Case GPSBabel#2 before. The existing 245 test is an example for Case GPSBabel#1 and has been fixed.

In nmea.cc, waypoints are allocated as NmeaWaypoint, which is a subclass of Waypoint. Later on those waypoints are deleted through a pointer to the base class in WaypointList::flush(). Found by gcc 9.2.1 with address sanitizer on Fedora 31. (./gpsbabel -i nmea -f ./reference/track/nmea -o gpx -F /tmp/gpsbabel.69273/nmea.gpx) ================================================================= ==70649==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x610000000340 in thread T0: object passed to delete has wrong type: size of the allocated type: 192 bytes; size of the deallocated type: 184 bytes. #0 0x7f210e8a70b5 in operator delete(void*, unsigned long) (/lib64/libasan.so.5+0x1110b5) GPSBabel#1 0x6edd68 in WaypointList::flush() /home/gpsbabel/gpsbabel.git/waypt.cc:742 GPSBabel#2 0x6e4ea3 in route_head::~route_head() /home/gpsbabel/gpsbabel.git/route.cc:391 GPSBabel#3 0x6e5929 in RouteList::flush() /home/gpsbabel/gpsbabel.git/route.cc:473 GPSBabel#4 0x6e29e8 in route_flush_all_tracks() /home/gpsbabel/gpsbabel.git/route.cc:180 GPSBabel#5 0x6e29fe in route_deinit() /home/gpsbabel/gpsbabel.git/route.cc:187 GPSBabel#6 0x74648f in main /home/gpsbabel/gpsbabel.git/main.cc:669 GPSBabel#7 0x7f210dd3c1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) GPSBabel#8 0x4104bd in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4104bd) 0x610000000340 is located 0 bytes inside of 192-byte region [0x610000000340,0x610000000400) allocated by thread T0 here: #0 0x7f210e8a59d7 in operator new(unsigned long) (/lib64/libasan.so.5+0x10f9d7) GPSBabel#1 0x484492 in gpgga_parse /home/gpsbabel/gpsbabel.git/nmea.cc:514 GPSBabel#2 0x489418 in nmea_parse_one_line /home/gpsbabel/gpsbabel.git/nmea.cc:996 GPSBabel#3 0x489bd3 in nmea_read /home/gpsbabel/gpsbabel.git/nmea.cc:1092 GPSBabel#4 0x41e409 in LegacyFormat::read() (/home/gpsbabel/gpsbabel.git/gpsbabel+0x41e409) GPSBabel#5 0x742667 in run /home/gpsbabel/gpsbabel.git/main.cc:301 GPSBabel#6 0x74647f in main /home/gpsbabel/gpsbabel.git/main.cc:666 GPSBabel#7 0x7f210dd3c1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) SUMMARY: AddressSanitizer: new-delete-type-mismatch (/lib64/libasan.so.5+0x1110b5) in operator delete(void*, unsigned long)

Found with afl and address sanitizer: AddressSanitizer:DEADLYSIGNAL ================================================================= ==2615627==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f54eaa53683 bp 0x7ffd187eed00 sp 0x7ffd187ee4b8 T0) ==2615627==The signal is caused by a READ memory access. ==2615627==Hint: address points to the zero page. #0 0x7f54eaa53682 in QString::chop(int) (/lib64/libQt5Core.so.5+0x13c682) GPSBabel#1 0x5e8d7f in gpgsa_parse /home/gpsbabel/gpsbabel.git/nmea.cc:744 GPSBabel#2 0x5e8d7f in nmea_parse_one_line /home/gpsbabel/gpsbabel.git/nmea.cc:1021 GPSBabel#3 0x5f0b6f in nmea_read /home/gpsbabel/gpsbabel.git/nmea.cc:1096 GPSBabel#4 0xc7e483 in run /home/gpsbabel/gpsbabel.git/main.cc:339 GPSBabel#5 0x4ced15 in main /home/gpsbabel/gpsbabel.git/main.cc:707 GPSBabel#6 0x7f54ea3f71a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) GPSBabel#7 0x4cffdd in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cffdd) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/lib64/libQt5Core.so.5+0x13c682) in QString::chop(int) ==2615627==ABORTING

In gbfgetstr(), when file->buff contains exactly file->buffsz characters including null termination, there is no room to append another character with strcat() in igc.cc Found by afl with address sanitizer: ==2082077==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000001f40 at pc 0x7f5a4e0da6cd bp 0x7ffd14ffe040 sp 0x7ffd14ffd7e8 WRITE of size 2 at 0x611000001f40 thread T0 #0 0x7f5a4e0da6cc (/lib64/libasan.so.5+0x9b6cc) GPSBabel#1 0x7327cf in data_read /home/gpsbabel/gpsbabel.git/igc.cc:334 GPSBabel#2 0xc7a3c1 in run /home/gpsbabel/gpsbabel.git/main.cc:339 GPSBabel#3 0x4cddf2 in main /home/gpsbabel/gpsbabel.git/main.cc:707 GPSBabel#4 0x7f5a4d5e51a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) GPSBabel#5 0x4cf00d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf00d) 0x611000001f40 is located 0 bytes to the right of 256-byte region [0x611000001e40,0x611000001f40)

%15.10f may produce more characters than what fits into the buffer of 32 bytes. Use std::isnan to avoid the sprintf. ==2133227==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff9bab5b20 at pc 0x7fc83fd95865 bp 0x7fff9bab57e0 sp 0x7fff9bab4f70 WRITE of size 38 at 0x7fff9bab5b20 thread T0 #0 0x7fc83fd95864 in vsprintf (/lib64/libasan.so.5+0x9e864) GPSBabel#1 0x7fc83fd95d1e in sprintf (/lib64/libasan.so.5+0x9ed1e) GPSBabel#2 0x767bbb in lowranceusr_parse_waypt /home/gpsbabel/gpsbabel.git/lowranceusr.cc:813 GPSBabel#3 0x77be54 in lowranceusr_parse_waypts /home/gpsbabel/gpsbabel.git/lowranceusr.cc:1116 GPSBabel#4 0x781151 in data_read /home/gpsbabel/gpsbabel.git/lowranceusr.cc:1658 GPSBabel#5 0xc7ac71 in run /home/gpsbabel/gpsbabel.git/main.cc:339 GPSBabel#6 0x4cdd62 in main /home/gpsbabel/gpsbabel.git/main.cc:707 GPSBabel#7 0x7fc83f29d1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) GPSBabel#8 0x4cef7d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cef7d)

When EOF or buffer limit is reached, the return buffer might not be zero-terminated. So add termination at the end of the buffer and read max sz - 1 bytes from file instead of sz bytes. ==2145172==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd2ba34e00 at pc 0x7f005812dbbd bp 0x7ffd2ba34860 sp 0x7ffd2ba34008 READ of size 1028 at 0x7ffd2ba34e00 thread T0 #0 0x7f005812dbbc (/lib64/libasan.so.5+0x67bbc) GPSBabel#1 0x557c62 in QString::fromUtf8(char const*, int) /usr/include/qt5/QtCore/qstring.h:572 GPSBabel#2 0x557c62 in QString::operator=(char const*) /usr/include/qt5/QtCore/qstring.h:706 GPSBabel#3 0x557c62 in mps_waypoint_r /home/gpsbabel/gpsbabel.git/mapsource.cc:571 GPSBabel#4 0x55edd4 in mps_read /home/gpsbabel/gpsbabel.git/mapsource.cc:1675 GPSBabel#5 0xc7adb1 in run /home/gpsbabel/gpsbabel.git/main.cc:339 GPSBabel#6 0x4ce082 in main /home/gpsbabel/gpsbabel.git/main.cc:707 GPSBabel#7 0x7f005766c1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) GPSBabel#8 0x4cf29d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf29d) Address 0x7ffd2ba34e00 is located in stack of thread T0 at offset 1344 in frame #0 0x556e8f in mps_waypoint_r /home/gpsbabel/gpsbabel.git/mapsource.cc:507 This frame has 6 object(s): [48, 56) 'wptdesc' (line 538) [80, 88) '<unknown>' [112, 120) '<unknown>' [144, 152) '<unknown>' [176, 276) 'tbuf' (line 508) [320, 1344) 'wptname' (line 509) <== Memory access at offset 1344 overflows this variable

The message_def array is 16 bytes long, make sure local_id stays within range, same as in other places in garmin_fit. Found by afl which managed to create a call to fit_parse_data_message with a header value of 0x1f: ==2927747==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000010a3bd8 at pc 0x000000a91bba bp 0x7ffd82584a90 sp 0x7ffd82584a80 READ of size 4 at 0x0000010a3bd8 thread T0 #0 0xa91bb9 in fit_parse_data /home/gpsbabel/gpsbabel.git/garmin_fit.cc:549 GPSBabel#1 0xa92ae3 in fit_parse_data_message /home/gpsbabel/gpsbabel.git/garmin_fit.cc:821 GPSBabel#2 0xa92ae3 in fit_parse_record /home/gpsbabel/gpsbabel.git/garmin_fit.cc:864 GPSBabel#3 0xa92ae3 in fit_read /home/gpsbabel/gpsbabel.git/garmin_fit.cc:884 GPSBabel#4 0xc7cdb1 in run /home/gpsbabel/gpsbabel.git/main.cc:339 GPSBabel#5 0x4ce142 in main /home/gpsbabel/gpsbabel.git/main.cc:707 GPSBabel#6 0x7f4f3b2651a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) GPSBabel#7 0x4cf35d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf35d)

* Fix off-by-one in nmea reader Found with afl and address sanitizer: AddressSanitizer:DEADLYSIGNAL ================================================================= ==2615627==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f54eaa53683 bp 0x7ffd187eed00 sp 0x7ffd187ee4b8 T0) ==2615627==The signal is caused by a READ memory access. ==2615627==Hint: address points to the zero page. #0 0x7f54eaa53682 in QString::chop(int) (/lib64/libQt5Core.so.5+0x13c682) #1 0x5e8d7f in gpgsa_parse /home/gpsbabel/gpsbabel.git/nmea.cc:744 #2 0x5e8d7f in nmea_parse_one_line /home/gpsbabel/gpsbabel.git/nmea.cc:1021 #3 0x5f0b6f in nmea_read /home/gpsbabel/gpsbabel.git/nmea.cc:1096 #4 0xc7e483 in run /home/gpsbabel/gpsbabel.git/main.cc:339 #5 0x4ced15 in main /home/gpsbabel/gpsbabel.git/main.cc:707 #6 0x7f54ea3f71a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #7 0x4cffdd in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cffdd) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/lib64/libQt5Core.so.5+0x13c682) in QString::chop(int) ==2615627==ABORTING * Fix heap buffer overflow in igc reader In gbfgetstr(), when file->buff contains exactly file->buffsz characters including null termination, there is no room to append another character with strcat() in igc.cc Found by afl with address sanitizer: ==2082077==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000001f40 at pc 0x7f5a4e0da6cd bp 0x7ffd14ffe040 sp 0x7ffd14ffd7e8 WRITE of size 2 at 0x611000001f40 thread T0 #0 0x7f5a4e0da6cc (/lib64/libasan.so.5+0x9b6cc) #1 0x7327cf in data_read /home/gpsbabel/gpsbabel.git/igc.cc:334 #2 0xc7a3c1 in run /home/gpsbabel/gpsbabel.git/main.cc:339 #3 0x4cddf2 in main /home/gpsbabel/gpsbabel.git/main.cc:707 #4 0x7f5a4d5e51a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #5 0x4cf00d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf00d) 0x611000001f40 is located 0 bytes to the right of 256-byte region [0x611000001e40,0x611000001f40) * Fix endless loop in mapsource The loop around gbfread needs a check for eof, otherwise it may never terminate with special input created by afl.

* Fix uninitialized warning in lowranceusr * Fix stack buffer overlow in lowranceusr %15.10f may produce more characters than what fits into the buffer of 32 bytes. Use std::isnan to avoid the sprintf. ==2133227==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff9bab5b20 at pc 0x7fc83fd95865 bp 0x7fff9bab57e0 sp 0x7fff9bab4f70 WRITE of size 38 at 0x7fff9bab5b20 thread T0 #0 0x7fc83fd95864 in vsprintf (/lib64/libasan.so.5+0x9e864) #1 0x7fc83fd95d1e in sprintf (/lib64/libasan.so.5+0x9ed1e) #2 0x767bbb in lowranceusr_parse_waypt /home/gpsbabel/gpsbabel.git/lowranceusr.cc:813 #3 0x77be54 in lowranceusr_parse_waypts /home/gpsbabel/gpsbabel.git/lowranceusr.cc:1116 #4 0x781151 in data_read /home/gpsbabel/gpsbabel.git/lowranceusr.cc:1658 #5 0xc7ac71 in run /home/gpsbabel/gpsbabel.git/main.cc:339 #6 0x4cdd62 in main /home/gpsbabel/gpsbabel.git/main.cc:707 #7 0x7fc83f29d1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #8 0x4cef7d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cef7d) * Ensure string is terminated mps_readstr in mapsource When EOF or buffer limit is reached, the return buffer might not be zero-terminated. So add termination at the end of the buffer and read max sz - 1 bytes from file instead of sz bytes. ==2145172==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd2ba34e00 at pc 0x7f005812dbbd bp 0x7ffd2ba34860 sp 0x7ffd2ba34008 READ of size 1028 at 0x7ffd2ba34e00 thread T0 #0 0x7f005812dbbc (/lib64/libasan.so.5+0x67bbc) #1 0x557c62 in QString::fromUtf8(char const*, int) /usr/include/qt5/QtCore/qstring.h:572 #2 0x557c62 in QString::operator=(char const*) /usr/include/qt5/QtCore/qstring.h:706 #3 0x557c62 in mps_waypoint_r /home/gpsbabel/gpsbabel.git/mapsource.cc:571 #4 0x55edd4 in mps_read /home/gpsbabel/gpsbabel.git/mapsource.cc:1675 #5 0xc7adb1 in run /home/gpsbabel/gpsbabel.git/main.cc:339 #6 0x4ce082 in main /home/gpsbabel/gpsbabel.git/main.cc:707 #7 0x7f005766c1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #8 0x4cf29d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf29d) Address 0x7ffd2ba34e00 is located in stack of thread T0 at offset 1344 in frame #0 0x556e8f in mps_waypoint_r /home/gpsbabel/gpsbabel.git/mapsource.cc:507 This frame has 6 object(s): [48, 56) 'wptdesc' (line 538) [80, 88) '<unknown>' [112, 120) '<unknown>' [144, 152) '<unknown>' [176, 276) 'tbuf' (line 508) [320, 1344) 'wptname' (line 509) <== Memory access at offset 1344 overflows this variable * Fix off-by-one list access in pcx reader * Prevent stack based buffer overflow in gdb reader * Prevent global buffer overflow in garmin_fit The message_def array is 16 bytes long, make sure local_id stays within range, same as in other places in garmin_fit. Found by afl which managed to create a call to fit_parse_data_message with a header value of 0x1f: ==2927747==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000010a3bd8 at pc 0x000000a91bba bp 0x7ffd82584a90 sp 0x7ffd82584a80 READ of size 4 at 0x0000010a3bd8 thread T0 #0 0xa91bb9 in fit_parse_data /home/gpsbabel/gpsbabel.git/garmin_fit.cc:549 #1 0xa92ae3 in fit_parse_data_message /home/gpsbabel/gpsbabel.git/garmin_fit.cc:821 #2 0xa92ae3 in fit_parse_record /home/gpsbabel/gpsbabel.git/garmin_fit.cc:864 #3 0xa92ae3 in fit_read /home/gpsbabel/gpsbabel.git/garmin_fit.cc:884 #4 0xc7cdb1 in run /home/gpsbabel/gpsbabel.git/main.cc:339 #5 0x4ce142 in main /home/gpsbabel/gpsbabel.git/main.cc:707 #6 0x7f4f3b2651a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #7 0x4cf35d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf35d)

GoogleCodeExporter added Priority-Medium Type-Review auto-migrated labels Jul 31, 2015

GPSBabelDeveloper closed this as completed Aug 1, 2015

stefan-a-bauer mentioned this issue Jan 14, 2016

mtk_logger mtk_parse_info: Fix track start for Holux devices #27

Merged

robertlipe mentioned this issue Mar 25, 2019

GPSBabel doesn't want to launch under Windows 10 #330

Closed

tsteven4 mentioned this issue May 13, 2019

Segmentation fault with Navin Minihomer #351

Closed

turepalsson mentioned this issue Aug 20, 2019

Garmin eTrex Vista HCX not detected on USB on Macos #381

Closed

ra1fh mentioned this issue Feb 4, 2020

Destructor of class Waypoint should be virtual #491

Closed

robertlipe mentioned this issue Jan 20, 2023

current_time() is not valid in gpsbabel_testmode() #993

Open

robertlipe mentioned this issue Jul 28, 2023

Reduce complexity of simplify filter #1149

Merged

This was referenced Oct 25, 2023

Update (now archived) Oozy Rat from Geocaching.com in 2023 #1196

Merged

fix unicsv reader truncation differences on i386/debian #1201

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

testing review system #1

testing review system #1

GoogleCodeExporter commented Jul 31, 2015

testing review system #1

testing review system #1

Comments

GoogleCodeExporter commented Jul 31, 2015