bugGNU Octave - Bugs: bug #54407, [octave forge] (io) Fix string...

 
 

bug #54407: [octave forge] (io) Fix string access in csv2ell

Submitter:  Orion Poplawski <opoplawski>
Submitted:  Sun 29 Jul 2018 11:32:03 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Open/Closed:  * Closed
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 03 Aug 2018 09:17:25 PM UTC, comment #5: 

I pushed your patch here after checking the built-in tests still pass:
http://hg.code.sf.net/p/octave/io/rev/d7f9bd65d2c6

As to the other modules, I didn't see any other occurrences of improper string indexing, which is not to say they aren't there :-) just that I didn't spot them.

Thanks for the patch, closing report.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 31 Jul 2018 08:14:06 AM UTC, comment #4: 

OK, I'll look into it. AFAIR (memory comes back slowly) there was something with the way the string length was tested this way, maybe EOLs or so (CR-LF).

Amyway thanks for reporting.

Title adapted (again)

Philip Nienhuis <philipnienhuis>
Group Member
Mon 30 Jul 2018 08:18:50 PM UTC, comment #3: 

Well, no - the issue is you are indexing str[k+1], and you need to make sure that k+1 < the length of str.

Orion Poplawski <opoplawski>
Mon 30 Jul 2018 08:16:27 PM UTC, comment #2: 

Tests were run, but they didn't trigger the error.  They were triggered in the octave-statistics package tests.

I haven't checked any of the other source files.

yeah, the "+1"s were concerning - but it eventually just seemed completely wrong without them.  Alternative would be to compare (k < len-1) which perhaps is a more common construct to compare a 0-based index to the length of the array.

Orion Poplawski <opoplawski>
Mon 30 Jul 2018 07:55:31 PM UTC, comment #1: 

(io maintainer here)

Thanks.
Did you run the tests? csv2ell does have a few regression tests included.
(I'm a bit worried about the "+1" -'s you added, hopefully they doesn't screw up the proper indexing).

Are those offending construct also in cell2csv, csvexplode and csvconcat?

Maybe such constructs also exist in he mapping package in the gdalread.cc module.

Note: I'm by no means proficient in C / C++. I've adapted those modules quite a bit over the years but that sort of functional programming was no so hard :-)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 29 Jul 2018 11:32:03 PM UTC, original submission:  

Fedora is starting to build with -D_GLIBCXX_ASSERTIONS which is exposing some incorrect C++ string accesses.  The attached patch fixes some aborts in octave-io.

Orion Poplawski <opoplawski>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #44655:  octave-io-string.patch added by opoplawski (1KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Updated the item)
  • -email is unavailable- added by opoplawski (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-28 siko1056 Summary[octave-forge](io) Fix string access in csv2ell [octave forge] (io) Fix string access in csv2ell
    2018-08-03 philipnienhuis StatusNone Fixed
        Open/ClosedOpen Closed
    2018-07-31 philipnienhuis Assigned toNone philipnienhuis
        Operating SystemGNU/Linux Any
        Summary(octave-forge[io] Fix string access in csv2ell [octave-forge](io) Fix string access in csv2ell
    2018-07-30 philipnienhuis Summaryio: Fix string access (octave-forge[io] Fix string access in csv2ell
    2018-07-29 opoplawski Attached File- Added octave-io-string.patch, #44655

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code