schwehr / generic-sensor-format

Sonar Generic Sensor Format (gsf) codec
Other
13 stars 8 forks source link

First issue with asan #40

Closed schwehr closed 9 years ago

schwehr commented 9 years ago
==9364== ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffccf47248c at pc 0x4b01ad bp 0x7ffccf471120 sp 0x7ffccf471118
READ of size 4 at 0x7ffccf47248c thread T0
    #0 0x4b01ac (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x4b01ac)
    #1 0x47e6e8 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x47e6e8)
    #2 0x480339 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x480339)
    #3 0x40632f (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x40632f)
    #4 0x47bee0 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x47bee0)
    #5 0x463597 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x463597)
    #6 0x463825 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x463825)
    #7 0x463b90 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x463b90)
    #8 0x46422a (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x46422a)
    #9 0x47c7c9 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x47c7c9)
    #10 0x4648fb (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x4648fb)
    #11 0x47dcc9 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x47dcc9)
    #12 0x2b6197ba6ec4 (/lib/x86_64-linux-gnu/libc-2.19.so+0x21ec4)
    #13 0x405568 (/usr/local/google/home/schwehr/src/generic-sensor-format/tests/read_gsf_2_9_test+0x405568)
Address 0x7ffccf47248c is located at offset 44 in frame <gsfDecodeSwathBathymetryPing> of T0's stack:
  This frame has 2 object(s):
    [32, 44) 'test_sizes'
    [96, 100) 'bytes_to_unpack'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
Shadow bytes around the buggy address:
  0x100019e86440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100019e86450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100019e86460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100019e86470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100019e86480: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
=>0x100019e86490: 00[04]f4 f4 f2 f2 f2 f2 04 f4 f4 f4 00 00 00 00
  0x100019e864a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100019e864b0: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 f4
  0x100019e864c0: f4 f4 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
  0x100019e864d0: 00 00 f1 f1 f1 f1 00 00 f4 f4 f3 f3 f3 f3 00 00
  0x100019e864e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:     fa
  Heap righ redzone:     fb
  Freed Heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==9364== ABORTING
schwehr commented 9 years ago

count is used to index into test_sizes[3]. The loop was <= 3, such that test_sizes[3] was past the end of the array.

--- a/src/gsf_dec.c
+++ b/src/gsf_dec.c
@@ -861,7 +861,7 @@ gsfDecodeSwathBathymetryPing(gsfSwathBathyPing *ping, unsigned char *sptr, GSF_F
         /* Verification check on the next sub record id and size. */
         sr_size = subrecord_size;
         count = 0;
-        while (((record_size - bytes - sr_size) > 4) && (count <= 3))
+        while (((record_size - bytes - sr_size) > 4) && (count < 3))
         {

             int test_sizes[3] = {1, 2, 4};
schwehr commented 9 years ago

I don't think my prior fix is correct. This might be a better fix. However, it's not clear what is really going on here.

--- a/src/gsf_dec.c
+++ b/src/gsf_dec.c
@@ -863,8 +863,7 @@ gsfDecodeSwathBathymetryPing(gsfSwathBathyPing *ping, unsigned char *sptr, GSF_F
         count = 0;
         while (((record_size - bytes - sr_size) > 4) && (count <= 3))
         {
-
-            int test_sizes[3] = {1, 2, 4};
+            int test_sizes[4] = {1, 2, 4, -1};
             int test_fs;

             memcpy(&ltemp, (p + sr_size), 4);