bugGNU Octave - Bugs: bug #45947, savepath can create a ~/.octaverc...

 
 

bug #45947: savepath can create a ~/.octaverc that crashes Octave at startup

Submitted by:  Mike Miller <mtmiller>
Submitted on:  Sat 12 Sep 2015 07:31:28 PM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Segfault, Bus Error, etc.
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 16 May 2016 04:18:22 AM UTC, comment #10:

No comments, so I went ahead and applied the second solution from comment #9. See http://hg.savannah.gnu.org/hgweb/octave/rev/764e586dca4b.

Fixed, closing report.

Rik <rik5>
Project Administrator
Thu 12 May 2016 12:46:30 AM UTC, comment #9:

It isn't so hard to do. One way to eliminate the variable trailing context is to have explicit rules. This would catch the majority of instances, and is probably good enough given the 80/20 rule. For example, catching "get|set" followed by 0,1,2 spaces and a then a parenthesis would be:

I've tested this and it works.

Alternatively, remove the trailing context from the Flex pattern, but then you need to strip out the whitespace yourself.

This pattern works to stop the segfault.

However, the HANDLE_IDENTIFIER macro needs to be changed since the whitespace is now included in yytext.

I changed the code clause where get_set is true to this

and it works ("make check" passes).

Rik <rik5>
Project Administrator
Tue 10 May 2016 09:51:11 PM UTC, comment #8:

The REJECT feature is not used explicitly, but is triggered by this rule in lex.ll:

The problem is the variable-length trailing context (the whitespace followed by an open paren).

If we can change the way that get and set are handled for classdef methods like "get.property_name" to avoid using this trailing context, then this problem should go away.

There is one more use of trailing context in the lexer, but it doesn't involve variable length matching, so it isn't a problem.

Probably we just need a completely different way of recognizing these get/set identifiers in this context.

John W. Eaton <jwe>
Project Administrator
Mon 14 Sep 2015 08:02:59 AM UTC, comment #7:

If our scanner/lexer can't be modified to parse larger strings (e.g. not using REJECT as explained here [1]), at least (1) parsing a too large string should not lead to a segfault, (2) the manual should mention this limitation.

[1] http://flex.sourceforge.net/manual/Diagnostics.html#Diagnostics

Pantxo Diribarne <pantxo>
Project Member
Mon 14 Sep 2015 05:55:58 AM UTC, comment #6:

Savepath is easy to fix (see comment #1) but the segfault whith long strings is more serious. From a quick search, I found that this is a lexer issue that e.g. Doxygen also has (same limit of 16000 characters see [1]).

[1] see item 13: http://www.star.bnl.gov/public/comp/sofi/doxygen/faq.html

Pantxo Diribarne <pantxo>
Project Member
Sun 13 Sep 2015 10:59:53 PM UTC, comment #5:

Yes, see bug #45308 for my motivation for opening this bug. A novice user may not even know about the existence of ~/.octaverc, and may not know exactly what path is set to when savepath is called, but it could inadvertently "break" their Octave installation without any indication of what went wrong.

Mike Miller <mtmiller>
Project Administrator
Sun 13 Sep 2015 09:31:07 PM UTC, comment #4:

Does anybody really use this function anyways? We have to have it for Matlab compatibility, but I don't think we need to spend much time perfecting it. A path of over 26,000 characters seems to me to be a problem the user should solve, rather than Octave.

Rik <rik5>
Project Administrator
Sun 13 Sep 2015 09:05:48 PM UTC, comment #3:

A simpler way to reproduce the same segfault:

For me it crashes above n of order 1e7.

Pantxo Diribarne <pantxo>
Project Member
Sun 13 Sep 2015 08:35:32 PM UTC, comment #2:

And for the segfault, I tried to copy the monolithic path in .octaverc from an editor, and paste it (around 26400 characters according to LibreOffice) in the terminal between two quotes and it also crashed Octave with the same error messages.

So it looks like long string inputs are not well treated in the terminal either.

Pantxo Diribarne <pantxo>
Project Member
Sun 13 Sep 2015 10:41:54 AM UTC, comment #1:

I can confirm this behavior.

Why not add each path entry individually (something like the attached patch)? Even though startup time is large, it works.

(file #34868)

Pantxo Diribarne <pantxo>
Project Member
Sat 12 Sep 2015 07:31:28 PM UTC, original submission:

The savepath() function stores the current load path in ~/.octaverc, but it stores the path as one long command. If the user's currently configured load path is huge, this will crash Octave on startup.

Test case:

Now exit and start Octave again. Octave crashes when trying to execute the long savepath command line with:

Mike Miller <mtmiller>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #34868:  savepath_test.patch added by pantxo (2KiB - text/x-diff)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by pantxo (Updated the item)
  • -unavailable- added by mtmiller (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 16 May 2016 04:18:22 AM UTCrik5StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Sun 08 May 2016 04:35:39 PM UTCpantxoDependencies-=>bugs #47876 is dependent
    Mon 19 Oct 2015 06:55:18 AM UTCpantxoDependencies-=>bugs #46245 is dependent
    Sun 13 Sep 2015 10:41:54 AM UTCpantxoAttached File-=>Added savepath_test.patch, #34868
      StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1