[PATCH] mynav bugfix

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] mynav bugfix

Ralf Horstmann
Hi,

found a few bugs in mynav reader by running the afl fuzzer. Below is
a fix.

Cheers,
Ralf

Index: mynav.cc
===================================================================
--- mynav.cc (revision 4941)
+++ mynav.cc (working copy)
@@ -63,7 +63,7 @@
   }
 
   // don't consider lines without latitude/longitude
-  if (fields.size() < fld_lat)
+  if (fields.size() <= fld_lat)
     return;
 
   // only type 1 and type 5 lines contain coordinates
@@ -76,7 +76,7 @@
 
   // This field is not present in .trc files, only in .ftn, so
   // ignore line if present and != 1
-  if (fields.size() >= fld_gps_valid) {
+  if (fields.size() > fld_gps_valid) {
     int val_gps_valid = fields.at(fld_gps_valid).trimmed().toInt(&ok);
     if (!ok || val_gps_valid != 1)
       return;
@@ -93,13 +93,13 @@
   wpt->latitude = val_lat;
   wpt->longitude = val_lon;
 
-  if (fields.size() >= fld_altitude) {
+  if (fields.size() > fld_altitude) {
     double val_alt = fields.at(fld_altitude).trimmed().toDouble(&ok);
     if (ok)
       wpt->altitude = val_alt;
   }
 
-  if (fields.size() >= fld_timestamp) {
+  if (fields.size() > fld_timestamp) {
     int val_time = fields.at(fld_timestamp).trimmed().toInt(&ok);
     if (ok)
       wpt->SetCreationTime(val_time);


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mynav bugfix

Robert Lipe-4
Thanx. Applied.  (by hand since inlining a diff in mail instead of attaching it rarely works.)

I infer you were using http://lcamtuf.coredump.cx/afl/ - that sounds like an interesting project.  I cringe at the thought of applying it to our binary formats, but if you have a recipe for using it to find bugs in GPSBabel, I'd be interested in hearing more about it it.

On Wed, Dec 10, 2014 at 3:25 PM, Ralf Horstmann <[hidden email]> wrote:
Hi,

found a few bugs in mynav reader by running the afl fuzzer. Below is
a fix.

Cheers,
Ralf

Index: mynav.cc
===================================================================
--- mynav.cc    (revision 4941)
+++ mynav.cc    (working copy)
@@ -63,7 +63,7 @@
   }

   // don't consider lines without latitude/longitude
-  if (fields.size() < fld_lat)
+  if (fields.size() <= fld_lat)
     return;

   // only type 1 and type 5 lines contain coordinates
@@ -76,7 +76,7 @@

   // This field is not present in .trc files, only in .ftn, so
   // ignore line if present and != 1
-  if (fields.size() >= fld_gps_valid) {
+  if (fields.size() > fld_gps_valid) {
     int val_gps_valid = fields.at(fld_gps_valid).trimmed().toInt(&ok);
     if (!ok || val_gps_valid != 1)
       return;
@@ -93,13 +93,13 @@
   wpt->latitude = val_lat;
   wpt->longitude = val_lon;

-  if (fields.size() >= fld_altitude) {
+  if (fields.size() > fld_altitude) {
     double val_alt = fields.at(fld_altitude).trimmed().toDouble(&ok);
     if (ok)
       wpt->altitude = val_alt;
   }

-  if (fields.size() >= fld_timestamp) {
+  if (fields.size() > fld_timestamp) {
     int val_time = fields.at(fld_timestamp).trimmed().toInt(&ok);
     if (ok)
       wpt->SetCreationTime(val_time);


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mynav bugfix

Ralf Horstmann
* Robert Lipe <[hidden email]> [2014-12-15 03:14]:
> Thanx. Applied.  (by hand since inlining a diff in mail instead of
> attaching it rarely works.)

Hm, usually that works for me. Maybe my mailer does something wrong.
 
> I infer you were using http://lcamtuf.coredump.cx/afl/ - that sounds like an
> interesting project.  I cringe at the thought of applying it to our binary
> formats, but if you have a recipe for using it to find bugs in GPSBabel, I'd
> be interested in hearing more about it it.

Correct, that's the fuzzer I used. It's fairly simple to use.

The first step is to instrument the build with afl-gcc and afl-g++ compiler
wrappers. The commands look like this (this is for OpenBSD, on Linux it might
be slightly different):

CC=afl-gcc CXX=afl-g++ CPPFLAGS=-I/usr/local/include CFLAGS=-L/usr/local/lib \
    QMAKE=/usr/local/bin/qmake4 ./configure
make

For mynav, I took a minimized valid input as starting point, as recommended by the
afl documentation. It was located in afl-input subdirectory for the example
command line below:

afl-fuzz -d -i afl-input -o afl-output ./gpsbabel -i mynav -f @@ -o gpx -F /dev/null

The interesting input files will be collected in afl-output/hangs and
afl-output/crashes.

With this setup, afl found another bug. Endless loop in this case. It's located
somewhere in the code that does unicode input processing. Although I used mynav
as input format, there are other modules affected as well that use gbfgetstr().

The input file is fairly simple, just 5 bytes long, see attached.
To reproduce:

gpsbabel -i mynav -f hang.trc -o gpx -F /dev/null
gpsbabel -i pcx -f hang.trc -o gpx -F /dev/null
gpsbabel -i vpl -f hang.trc  -o gpx -F /dev/null

The code involved is a bit obscure, so I can't provide a patch for this at the
moment.

Cheers,
Ralf

>
> On Wed, Dec 10, 2014 at 3:25 PM, Ralf Horstmann <[hidden email]>
> wrote:
>
> > Hi,
> >
> > found a few bugs in mynav reader by running the afl fuzzer. Below is a fix.
> >
> > Cheers, Ralf
> >
> > Index: mynav.cc
> > =================================================================== ---
> > mynav.cc    (revision 4941) +++ mynav.cc    (working copy) @@ -63,7 +63,7
> > @@ }
> >
> >    // don't consider lines without latitude/longitude -  if (fields.size()
> >    < fld_lat) +  if (fields.size() <= fld_lat) return;
> >
> >    // only type 1 and type 5 lines contain coordinates @@ -76,7 +76,7 @@
> >
> >    // This field is not present in .trc files, only in .ftn, so // ignore
> >    line if present and != 1 -  if (fields.size() >= fld_gps_valid) { +  if
> >    (fields.size() > fld_gps_valid) { int val_gps_valid =
> >    fields.at(fld_gps_valid).trimmed().toInt(&ok); if (!ok || val_gps_valid
> >    != 1) return; @@ -93,13 +93,13 @@ wpt->latitude = val_lat;
> >    wpt->longitude = val_lon;
> >
> > -  if (fields.size() >= fld_altitude) { +  if (fields.size() >
> > fld_altitude) { double val_alt =
> > fields.at(fld_altitude).trimmed().toDouble(&ok); if (ok) wpt->altitude =
> > val_alt; }
> >
> > -  if (fields.size() >= fld_timestamp) { +  if (fields.size() >
> > fld_timestamp) { int val_time =
> > fields.at(fld_timestamp).trimmed().toInt(&ok); if (ok)
> > wpt->SetCreationTime(val_time);
> >
> >
> >
> > ------------------------------------------------------------------------------
> > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from
> > Actuate! Instantly Supercharge Your Business Reports and Dashboards with
> > Interactivity, Sharing, Native Excel Exports, App Integration & more Get
> > technology previously reserved for billion-dollar corporations, FREE
> >
> > http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
> > _______________________________________________ Gpsbabel-code mailing list
> > http://www.gpsbabel.org [hidden email]
> > https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
> >

> ------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk

> _______________________________________________
> Gpsbabel-code mailing list  http://www.gpsbabel.org
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code



Gruss, Ralf.

--
[hidden email] GPG 0xE9ED33BA http://pgp.mit.edu

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code

hang.trc (10 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mynav bugfix

Robert Lipe-4
Great stuff.  Thank you.  I'm both intrigued and horrified that it can find bugs like that.

This is actually an interesting bug. I have a fix in hand that I'll submit tomorrow.

On Mon, Dec 15, 2014 at 3:33 PM, Ralf Horstmann <[hidden email]> wrote:
* Robert Lipe <[hidden email]> [2014-12-15 03:14]:
> Thanx. Applied.  (by hand since inlining a diff in mail instead of
> attaching it rarely works.)

Hm, usually that works for me. Maybe my mailer does something wrong.

> I infer you were using http://lcamtuf.coredump.cx/afl/ - that sounds like an
> interesting project.  I cringe at the thought of applying it to our binary
> formats, but if you have a recipe for using it to find bugs in GPSBabel, I'd
> be interested in hearing more about it it.

Correct, that's the fuzzer I used. It's fairly simple to use.

The first step is to instrument the build with afl-gcc and afl-g++ compiler
wrappers. The commands look like this (this is for OpenBSD, on Linux it might
be slightly different):

CC=afl-gcc CXX=afl-g++ CPPFLAGS=-I/usr/local/include CFLAGS=-L/usr/local/lib \
    QMAKE=/usr/local/bin/qmake4 ./configure
make

For mynav, I took a minimized valid input as starting point, as recommended by the
afl documentation. It was located in afl-input subdirectory for the example
command line below:

afl-fuzz -d -i afl-input -o afl-output ./gpsbabel -i mynav -f @@ -o gpx -F /dev/null

The interesting input files will be collected in afl-output/hangs and
afl-output/crashes.

With this setup, afl found another bug. Endless loop in this case. It's located
somewhere in the code that does unicode input processing. Although I used mynav
as input format, there are other modules affected as well that use gbfgetstr().

The input file is fairly simple, just 5 bytes long, see attached.
To reproduce:

gpsbabel -i mynav -f hang.trc -o gpx -F /dev/null
gpsbabel -i pcx -f hang.trc -o gpx -F /dev/null
gpsbabel -i vpl -f hang.trc  -o gpx -F /dev/null

The code involved is a bit obscure, so I can't provide a patch for this at the
moment.

Cheers,
Ralf

>
> On Wed, Dec 10, 2014 at 3:25 PM, Ralf Horstmann <[hidden email]>
> wrote:
>
> > Hi,
> >
> > found a few bugs in mynav reader by running the afl fuzzer. Below is a fix.
> >
> > Cheers, Ralf
> >
> > Index: mynav.cc
> > =================================================================== ---
> > mynav.cc    (revision 4941) +++ mynav.cc    (working copy) @@ -63,7 +63,7
> > @@ }
> >
> >    // don't consider lines without latitude/longitude -  if (fields.size()
> >    < fld_lat) +  if (fields.size() <= fld_lat) return;
> >
> >    // only type 1 and type 5 lines contain coordinates @@ -76,7 +76,7 @@
> >
> >    // This field is not present in .trc files, only in .ftn, so // ignore
> >    line if present and != 1 -  if (fields.size() >= fld_gps_valid) { +  if
> >    (fields.size() > fld_gps_valid) { int val_gps_valid =
> >    fields.at(fld_gps_valid).trimmed().toInt(&ok); if (!ok || val_gps_valid
> >    != 1) return; @@ -93,13 +93,13 @@ wpt->latitude = val_lat;
> >    wpt->longitude = val_lon;
> >
> > -  if (fields.size() >= fld_altitude) { +  if (fields.size() >
> > fld_altitude) { double val_alt =
> > fields.at(fld_altitude).trimmed().toDouble(&ok); if (ok) wpt->altitude =
> > val_alt; }
> >
> > -  if (fields.size() >= fld_timestamp) { +  if (fields.size() >
> > fld_timestamp) { int val_time =
> > fields.at(fld_timestamp).trimmed().toInt(&ok); if (ok)
> > wpt->SetCreationTime(val_time);
> >
> >
> >
> > ------------------------------------------------------------------------------
> > Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from
> > Actuate! Instantly Supercharge Your Business Reports and Dashboards with
> > Interactivity, Sharing, Native Excel Exports, App Integration & more Get
> > technology previously reserved for billion-dollar corporations, FREE
> >
> > http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
> > _______________________________________________ Gpsbabel-code mailing list
> > http://www.gpsbabel.org [hidden email]
> > https://lists.sourceforge.net/lists/listinfo/gpsbabel-code
> >

> ------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk

> _______________________________________________
> Gpsbabel-code mailing list  http://www.gpsbabel.org
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/gpsbabel-code



Gruss, Ralf.

--
[hidden email] GPG 0xE9ED33BA http://pgp.mit.edu

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Gpsbabel-code mailing list  http://www.gpsbabel.org
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gpsbabel-code