patchGNU Octave - Patches: patch #7964, Possible overflow in terminal...

 
 

patch #7964: Possible overflow in terminal buffer

Submitter:  Andre da Costa Barros <andrecbarros>
Submitted:  Tue 05 Mar 2013 02:04:04 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 03 Apr 2013 10:35:01 PM UTC, comment #4: 

I applied the patch, crediting Andre, in this changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/ad070b353e59)

Rik <rik5>
Group administrator
Wed 03 Apr 2013 08:46:40 PM UTC, comment #3: 

Yes, 1 byte is enough. It is just a habit I got from old times when word aligned variables was a good practice. Now the compilers do it by default I think.

Andre da Costa Barros <andrecbarros>
Tue 02 Apr 2013 08:22:50 AM UTC, comment #2: 

The extra byte on the buffer just needs to be a single character, not a whole int, i.e., buf[4096 + 1].

Dan Sebald <sebald>
Tue 02 Apr 2013 08:18:09 AM UTC, comment #1: 

This post should have gone under "Bugs".  Patches are for non-bug related items.

This definitely looks like a bug if "len" can be 4096. buf[len] would then be one beyond the end of the buffer.  I'd say apply the patch as a quick stopgap.

However, I don't know why a buffer size of 4096 is put on the stack.  A buffer that size should be on the heap.  Putting a listener to sleep for thirty seconds seems odd as well.  We try to not do such polling loop strategies, especially on the listening side.  This should be done with signals/slots.  I'm guessing the thirty second wait described in

https://savannah.gnu.org/patch/?7949

is related to this.

Dan Sebald <sebald>
Tue 05 Mar 2013 02:04:04 PM UTC, original submission:  

The possible offending code is displayed below with arrows to highlight where trouble may arise.

  void SelfListener::run() {
->    char buf[4096];
      int len;
      bool running = true;
      while(running) {
           while((len = ::read(_a, buf, 4096)) > 0) {
->            buf[len] = 0; // Just in case.
              emit recvData(buf, len);
              msleep(30);
           }
           if(len < 0)
             running = false;
      }
  }



Andre da Costa Barros <andrecbarros>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #27568:  octave-buffer-overflow.patch added by andrecbarros (389B - text/x-patch - buffer size padding)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by andrecbarros (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 logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-04-03 rik5 StatusNone Done
        Open/ClosedOpen Closed
    2013-03-05 andrecbarros Attached File- Added octave-buffer-overflow.patch, #27568

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code