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

 
 

patch #7964: Possible overflow in terminal buffer

Submitted by:  Andre da Costa Barros <andrecbarros>
Submitted on:  Tue 05 Mar 2013 02:04:04 PM UTC  
 
Category: NonePriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: NoneOpen/Closed: Closed

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

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>
Project 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 File(s):
   
   
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
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by sebald (Posted a comment)
  • -unavailable- added by andrecbarros (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 logged-in users can vote.

     

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

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 03 Apr 2013 10:35:01 PM UTCrik5StatusNone=>Done
      Open/ClosedOpen=>Closed
    Tue 05 Mar 2013 02:04:04 PM UTCandrecbarrosAttached File-=>Added octave-buffer-overflow.patch, #27568

    Back to the top


    Powered by Savane 3.1-cleanup