[GRASS-dev] [GRASS GIS] #1311: DebCheck QA: Array index is out of bounds

GRASS GRASS developer
1

#1311: DebCheck QA: Array index is out of bounds
------------------------------+---------------------------------------------
Reporter: hamish | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.2
Component: Compiling | Version: 6.4.0
Keywords: r.flow, v.in.dwg | Platform: Linux
      Cpu: Unspecified |
------------------------------+---------------------------------------------
Hi,

Debian's run of the Cpp check tool automatically found a number of C-code
errors (or
potential errors) which need to be reviewed by hand. See grass-dev ML
thread
of 13 Jan 2011.

about:
  http://www.linuxjournal.com/content/daca-could-mean-less-bugs-debian
the list of probably-bugs in the 6.4.0 C/C++ code:
   http://qa.debian.org/daca/cppcheck/squeeze/grass_6.4.0~rc6+42329-3.html
CLI analysis program (not Debian specific):
   http://cppcheck.wiki.sourceforge.net

I have split those 154 hits into 16 classes, and will report each class in
an individual bug report.

Run against releasebranch6_4 r42329 (including all patches up to, but not
beyond, 6.4.0-final)

This bug report is for: '''Array index is out of bounds'''

  * ./raster/r.flow/precomp.c:154 [error] - Array index -1 is out of bounds
  * ./raster/r.flow/precomp.c:158 [error] - Array index -1 is out of bounds
  * ./raster/r.flow/precomp.c:161 [error] - Array index -1 is out of bounds
  * ./raster/r.flow/precomp.c:163 [error] - Array index -1 is out of bounds
  * ./raster/r.flow/precomp.c:165 [error] - Array index -1 is out of bounds
  * ./vector/v.in.dwg/entity.c:516 [error] - Array 'tempdouble[2]' index 2
out of bounds
  * ./vector/v.in.dwg/entity.c:517 [error] - Array 'tempwidth[2]' index 2
out of bounds

Modules: r.flow, v.in.dwg

thanks,
Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1311&gt;
GRASS GIS <http://grass.osgeo.org>

2

#1311: DebCheck QA: Array index is out of bounds
------------------------------+---------------------------------------------
Reporter: hamish | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.2
Component: Compiling | Version: 6.4.0
Keywords: r.flow, v.in.dwg | Platform: Linux
      Cpu: Unspecified |
------------------------------+---------------------------------------------

Comment(by hamish):

suggested patch for v.in.dwg:
{{{
Index: entity.c

--- entity.c (revision 46424)
+++ entity.c (working copy)
@@ -293,7 +293,7 @@
      PAD_ENT_HDR adenhd2;
      PAD_ENT aden2;
      OdaLong il;
- double tempdouble[2], tempbulge, tempwidth[2];
+ double tempdouble[3], tempbulge, tempwidth[3];
      double x, y, z, ang;
      PAD_BLKH adblkh;
      int layer_found = 1;
}}}

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1311#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

3

#1311: DebCheck QA: Array index is out of bounds
------------------------------+---------------------------------------------
Reporter: hamish | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.2
Component: Compiling | Version: 6.4.0
Keywords: r.flow, v.in.dwg | Platform: Linux
      Cpu: Unspecified |
------------------------------+---------------------------------------------

Comment(by neteler):

Perhaps v.in.dwg becomes superfluous with
http://trac.osgeo.org/gdal/wiki/Release/1.9.0-News

DWG driver:
  * New for GDAL/OGR 1.9.0
  * Read DWG files through the use of Open Design Alliance Teigha Libraries

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1311#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

4

#1311: DebCheck QA: Array index is out of bounds
------------------------------+---------------------------------------------
Reporter: hamish | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.2
Component: Compiling | Version: 6.4.0
Keywords: r.flow, v.in.dwg | Platform: Linux
      Cpu: Unspecified |
------------------------------+---------------------------------------------

Comment(by hamish):

Replying to [comment:2 neteler]:
> Perhaps v.in.dwg becomes superfluous with
http://trac.osgeo.org/gdal/wiki/Release/1.9.0-News
>
> DWG driver:
> * New for GDAL/OGR 1.9.0
> * Read DWG files through the use of Open Design Alliance Teigha
Libraries

n.b. the Open Design Alliance is just the new name for the same OpenDWG
group, with source code only available to members, so same codebase and
license issues, but the development burden is outsourced to GDAL instead
of GRASS, which is not entirely a bad thing. (does the OGR lib have full
3D support etc?)

LibreDWG seems to be on its way:
   http://www.gnu.org/software/libredwg/

perhaps ask the BRL-CAD project for advice? (http://brlcad.org BRL-CAD
and GRASS are both about the same age and share very similar origins)

Hamish

ps- v.in.dwg patch now applied in trunk and devbr6

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1311#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

5

#1311: DebCheck QA: Array index is out of bounds
------------------------------+---------------------------------------------
Reporter: hamish | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.2
Component: Compiling | Version: 6.4.0
Keywords: r.flow, v.in.dwg | Platform: Linux
      Cpu: Unspecified |
------------------------------+---------------------------------------------

Comment(by hamish):

I knew it sounded familiar:

https://trac.osgeo.org/grass/browser/grass-addons/grass7/vector/v.in.redwg

http://lists.gnu.org/archive/html/info-libredwg/2010-08/msg00000.html

:slight_smile:

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1311#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

6

#1311: DebCheck QA: Array index is out of bounds
------------------------------+---------------------------------------------
Reporter: hamish | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 6.4.3
Component: Compiling | Version: 6.4.0
Keywords: r.flow, v.in.dwg | Platform: Linux
      Cpu: Unspecified |
------------------------------+---------------------------------------------
Changes (by hamish):

  * milestone: 6.4.2 => 6.4.3

Comment:

v.in.dwg patch backported to relbr64 in r52908. (done)

---

wrt r.flow, the offending code allowing array index of -1 is:

mem.h:
{{{
#define get(l, row, col) \
     (parm.seg ? \
         (segment_get(l.seg, &v, row + l.row_offset, col + l.col_offset) <
1 ? \
           (sprintf(string,"r.flow: cannot read segment file for
%s",l.name),\
            G_fatal_error(string)) : \
          v) : \
         l.buf[row][col])

#define put(l, row, col, w) \
     (parm.seg ? \
         (v = w, \
          segment_put(l.seg, &v, row + l.row_offset, col + l.col_offset) <
1 ? \
           (sprintf(string,"r.flow: cannot write segment file for
%s",l.name), \
            G_fatal_error(string)) : \
          0) : \
         (l.buf[row][col] = w))
}}}

precomp.c:
{{{
static void interpolate_border(void)
{
     int i, r = region.rows, c = region.cols;

     for (i = 0; i < c; i++) {
         put(el, -1, i, get(el, 0, i) * 2 - get(el, 1, i));
         put(el, r, i, get(el, r - 1, i) * 2 - get(el, r - 2, i));
     }
     for (i = 0; i < r; i++) {
         put(el, i, -1, get(el, i, 0) * 2 - get(el, i, 1));
         put(el, i, c, get(el, i, c - 1) * 2 - get(el, i, c - 2));
     }
     put(el, -1, -1, 3 * get(el, 0, 0) - get(el, 0, 1) - get(el, 1, 0));
     put(el, -1, c,
         3 * get(el, 0, c - 1) - get(el, 0, c - 2) - get(el, 1, c - 1));
     put(el, r, -1,
         3 * get(el, r - 1, 0) - get(el, r - 2, 0) - get(el, r - 1, 1));
     put(el, r, c,
         3 * get(el, r - 1, c - 1) - get(el, r - 2, c - 1) - get(el, r - 1,
                                                                 c - 2));
}
}}}

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1311#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>