bugGNU Octave - Bugs: bug #59094, Possible integer overflow in...


bug #59094: Possible integer overflow in __ode15__.cc

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Thu 10 Sep 2020 03:40:06 PM UTC
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)

Comment Type & Canned Response:


Tue 06 Oct 2020 06:31:08 PM UTC, comment #4: 

I manually undefined HAVE_SUNSPARSEMATRIX_REALLOCATE to check if the code actually runs (which it does).

Closing as fixed.

Markus Mützel <mmuetzel>
Project Member
Thu 01 Oct 2020 06:31:21 AM UTC, comment #3: 

I pushed the change here:

We could add additional overloads for char, unsigned char and signed char as we need them.
The same for the other operations (see comment #2).

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Sat 19 Sep 2020 11:24:30 AM UTC, comment #2: 

The attached patch adds functions that check whether multiplication of two "similar" integers would overflow. These functions wrap around macros provided by gnulib's `intprops` module.

It also adds the necessary check to `__ode15__.cc`.

The rules for integer multiplication between "dis-similar" integers and for function dispatching in C++ might differ (I haven't checked).
But imho it would make code easier to understand (and possibly avoid unintended behavior) if we explicitly casted to a common integer type before such kinds of operation anyway. So I don't think it is necessary to add functions that check integer overflow on operations between "dis-similar" types (like "int type" times "unsigned long int type").

If needed, we could add similar check functions for other potentially critical operations.
Checks are provided by gnulib for:

  • addition
  • subtraction
  • negation
  • multiplication
  • division
  • remainder
  • left shift

See also:

Adding jwe to CC because he made the original proposition.

(file #49819)

Markus Mützel <mmuetzel>
Project Member
Mon 14 Sep 2020 04:36:15 PM UTC, comment #1: 

With the change for bug #55905, the possible integer overflow is mitigated on systems with a version of SUNDIALS that provides "SUNSparseMatrix_Reallocate".

Markus Mützel <mmuetzel>
Project Member
Thu 10 Sep 2020 03:40:06 PM UTC, original submission:  

It is possible that the multiplication m_num*m_num in the following line in _ode15_.cc can overflow:

        m_sunJacMatrix = SUNSparseMatrix (m_num, m_num, m_num*m_num, CSC_MAT);


jwe suggested in bug #58795:

> Gnulib has macros for checking whether integer operations overflow, or we could (conditionally) use things like __builtin_mul_overflow to multiply dimensions and check for overflow.
> I recently noticed that one of the Octave dependencies (maybe it was Sundials?) was using the portable snippets library for this job.  Perhaps we could use that or at least borrow ideas from it.  See, for example,  https://github.com/nemequ/portable-snippets/tree/master/safe-math
> I hope we can refactor so that we limit the number of places where checks like this are needed.

The gnulib macros are probably the ones in "intprops.h":

IIUC, we try to avoid including gnulib headers directly. Instead we have wrappers for the functions we want to use.
I don't know how to do something similar with these macros.
Is it ok to include that header directly?

Markus Mützel <mmuetzel>
Project Member


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

Attach Files:

Attached Files
file #49819:  bug59094_ode15_integer_overflow.patch added by mmuetzel (10KiB - application/octet-stream)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by mmuetzel
  • -email is unavailable- added by mmuetzel (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 project members can vote.





    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-10-06 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-10-01 mmuetzel StatusPatch Submitted Ready For Test
    2020-09-19 mmuetzel Attached File- Added bug59094_ode15_integer_overflow.patch, #49819
        StatusNone Patch Submitted
        Carbon-Copy- Added jwe

    Back to the top

    Powered by Savane 3.9