Opened 8 years ago

Closed 4 years ago

#1430 closed defect (fixed)

Buffer overrun in vector/diglib dig__fread_port_L with big-endian negative values on LP64 systems

Reported by: rroliver Owned by: grass-dev@…
Priority: normal Milestone: 6.4.4
Component: Vector Version: svn-trunk
Keywords: diglib portable LP64 big-endian Cc:
CPU: All Platform: Unspecified

Description

The code in digfread_port_L is broken for handling big endian negative numbers on systems where sizeof(long) != PORT_LONG.

Presently the code fails to negate the value and will write 4 bytes past the end of the buffer.

Problem exists for all grass versions through to trunk...

Patch attached (patch against 6.4.1)

Attachments (1)

grass-6.4.1-diglib_fix_buffer_overrun-1.patch (1.2 KB) - added by rroliver 8 years ago.
Patch to fix buffer overrun in digfread_port_L

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by rroliver

Patch to fix buffer overrun in digfread_port_L

comment:1 in reply to:  description Changed 8 years ago by mmetz

Replying to rroliver:

The code in dig__fread_port_L is broken for handling big endian negative numbers on systems where sizeof(long) != PORT_LONG.

Presently the code fails to negate the value and will write 4 bytes past the end of the buffer.

Problem exists for all grass versions through to trunk...

The patch seems to make sense but there are more subtle bugs in lib/diglib/portable.c related to big endian systems. Can you test grass7 (trunk)? I have applied your patch plus some more modifications in trunk in r47992. After testing, they can be backported to 6.5. and 6.4.2.

Markus M

comment:2 Changed 6 years ago by hamish

Replying to mmetz in #2085:

It seems that r47992 needs to be backported (see also #1430).

ok, I guess the portable.c part of r56890 wants backporting too?

https://trac.osgeo.org/grass/changeset/56890/grass/trunk/lib/vector/diglib/portable.c

actually the backport is not so simple since there were a number of other LFS changes to the file in trunk since r32526:

https://trac.osgeo.org/grass/log/grass/trunk/lib/vector/diglib/portable.c

thanks, Hamish

comment:3 in reply to:  2 ; Changed 6 years ago by mmetz

Replying to hamish:

Replying to mmetz in #2085:

It seems that r47992 needs to be backported (see also #1430).

ok, I guess the portable.c part of r56890 wants backporting too?

Right, done in r57855,6.

comment:4 in reply to:  3 ; Changed 6 years ago by hamish

Replying to mmetz:

Replying to hamish:

Replying to mmetz in #2085:

It seems that r47992 needs to be backported (see also #1430).

ok, I guess the portable.c part of r56890 wants backporting too?

Right, done in r57855,6.

a sincere thank you, but please let changes to the core libraries prove themselves in devbr6 for a couple weeks before backporting into relbr64.

thanks, Hamish

comment:5 in reply to:  4 Changed 6 years ago by mmetz

Replying to hamish:

Replying to mmetz:

Replying to hamish:

Replying to mmetz in #2085:

It seems that r47992 needs to be backported (see also #1430).

ok, I guess the portable.c part of r56890 wants backporting too?

Right, done in r57855,6.

a sincere thank you, but please let changes to the core libraries prove themselves in devbr6 for a couple weeks before backporting into relbr64.

Apparently, this bug appeared in the official testing environment of Debian with the officially included GRASS version 6.4.3 on big endian hardware. I guess that it will take 3 - 4 years until my change will be tested in the same testing environment (Debian on big endian hardware with GRASS version 6.4.4, yet to be released and after that to be included into Debian).

We know that GRASS 7 compiles successfully on big endian hardware with AIX as OS. Therefore I am confident that my change will fix the reported bug. It would be preferable if a GRASS developer would have access to big endian hardware with Linux or BSD as OS and would regularly test svn versions of GRASS on such a system.

Further more, I would like to see the development branch of GRASS to be deleted.

comment:6 in reply to:  4 Changed 6 years ago by mmetz

Replying to hamish:

Replying to mmetz:

Replying to hamish:

Replying to mmetz in #2085:

It seems that r47992 needs to be backported (see also #1430).

ok, I guess the portable.c part of r56890 wants backporting too?

Right, done in r57855,6.

a sincere thank you, but please let changes to the core libraries prove themselves in devbr6 for a couple weeks before backporting into relbr64.

The change works in relbr64 with Fedora for ppc64. It seems that devbr6 will not get tested any time soon.

comment:7 Changed 4 years ago by martinl

Milestone: 6.4.6

comment:8 Changed 4 years ago by neteler

Milestone: 6.4.66.4.4
Resolution: fixed
Status: newclosed

Since nothing appears to be left open here since r57856 addressed relbranch64 2 years ago, closing.

Note: See TracTickets for help on using tickets.