bugGNU Screen - Bugs: bug #50197, out of bounds write when...

 
 

bug #50197: out of bounds write when dimensions are still 0, 0

Submitted by:  None
Submitted on:  Tue 31 Jan 2017 01:59:11 PM UTC  
 
Category: Crash/Freeze/InfloopSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: ClosedRelease: 4.4.0
Fixed Release: 4.5.1Planned Release: None
Work Required: None

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

Wed 01 Feb 2017 03:02:32 PM UTC, comment #2:

Thanks for detailed bug report and analysis.

Apparently it was already "fixed" on master branch by making Z0width and Z1width really constant.

cherry-picked to screen-v4 branch.

http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=a8c0d68f669eb17a2bf9d8d97f834f5cabb1dad5

Write string already has dimension check
if (cols > 0 && rows > 0)
{

Amadeusz Sławiński <amade>
Project Administrator
Wed 01 Feb 2017 08:47:02 AM UTC, comment #1:

More investigation yielded some more insights:
- rows/cols is zero because of an earlier escape sequence (\E[?1;3;4;6l) which is "column mode" and is supposed to initialize to either columns of 80 or 132, see e.g. http://vt100.net/docs/vt220-rm/chapter4.html, source code snippet:
- the numbers 80 and 132 are stored in the "constants" Z0width and Z1width which are to be set by InitTermcap, source code snippet:
- since we are starting the screen in detached mode, display is 0 and InitTermcap is never called:
So in summary I think there are at least two issues:
- when started in detached mode, some initialization is missing (Z0width, Z1width)
- WriteString should check for 0 dimensions and not call low-level functions which would do an out of bounds array access

Anonymous
Tue 31 Jan 2017 01:59:11 PM UTC, original submission:

Program received signal SIGSEGV, Segmentation fault.
0x080510da in MFixLine (p=p@entry=0x80cb2d8, y=y@entry=-1, mc=mc@entry=0x80cd830) at ansi.c:2371
2371 ansi.c: No such file or directory.
(gdb) bt
#0 0x080510da in MFixLine (p=p@entry=0x80cb2d8, y=y@entry=-1, mc=mc@entry=0x80cd830) at ansi.c:2371
#1 0x08051a7a in MPutChar (p=0x80cb2d8, c=0x80cd830, x=-1, y=-1) at ansi.c:2723
#2 0x08057e61 in WriteString (wp=0x0, buf=0xffb8c453 "\033[1;27H\r\n", len=175) at ansi.c:869
#3 0x08067868 in win_readev_fn (ev=0x80cb2e4, data=0x80cb2d8 "") at window.c:1942
#4 0x08090630 in sched () at sched.c:237
#5 0x0804c463 in main (ac=<optimized out>, av=<optimized out>) at screen.c:1487
(gdb) frame 0
#0 0x080510da in MFixLine (p=p@entry=0x80cb2d8, y=y@entry=-1, mc=mc@entry=0x80cd830) at ansi.c:2371
2371 if (mc->attr && ml->attr == null)
(gdb) up
#1 0x08051a7a in MPutChar (p=0x80cb2d8, c=0x80cd830, x=-1, y=-1) at ansi.c:2723
2723 MFixLine(p, y, c);
(gdb) up
#2 0x08057e61 in WriteString (wp=0x0, buf=0xffb8c453 "\033[1;27H\r\n", len=175) at ansi.c:869
869 MPutChar(curr, &curr->w_rend, curr->w_x, curr->w_y);
(gdb) l
864 curr->w_x++;
865 }
866 }
867 else if (curr->w_x == cols - 1)
868 {
869 MPutChar(curr, &curr->w_rend, curr->w_x, curr->w_y);
870 LPutChar(&curr->w_layer, &curr->w_rend, curr->w_x, curr->w_y);
871 if (curr->w_wrap)
872 curr->w_x++;
873 }
(gdb) p cols
$3 = 0
(gdb) p rows
$4 = 0

As one can see rows/cols are both 0 valued but are used for range index computations. It seems when a screen is started in detached mode those variables are never set to reasonable values (like 25, 80).
Above source code is actually from 4.2.1 as it was easier in Debian to retrieve the source from that version, yet the error also occurs in 4.4.0.

Reproduce:
Terminal 1:
$ xxd bad_sequence
00000000: 1b5b 721b 5b6d 1b5b 324a 1b5b 481b 5b3f .[r.[m.[2J.[H.[?
00000010: 3768 1b5b 3f31 3b33 3b34 3b36 6c1b 5b3b 7h.[?1;3;4;6l.[;
00000020: 481b 5b32 4a00 0000 0000 0000 0000 0000 H.[2J...........
00000030: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00000040: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00000050: 0000 0000 0000 001b 5b3b 481b 5b32 4a00 ........[;H.[2J.
00000060: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00000070: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00000080: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00000090: 001b 5b3b 376d 1b28 301b 5b31 3b31 3048 ..[;7m.(0.[1;10H
000000a0: 0000 0000 0078 1b5b 313b 3237 480a .....x.[1;27H.
$ screen -d -m bash -c 'sleep 10; cat bad_sequence; sleep 999'
Terminal 2 ("continue" in gdb and wait for the crash in less than 10 seconds)
$ gdb -p $(pgrep screen)

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #39625:  bad_sequence added by None (174B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by amade (Posted a comment)
  •  

    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 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 29 Jun 2017 10:16:13 AM UTCamadeOpen/ClosedOpen=>Closed
      Fixed ReleaseNone=>4.5.1
    Wed 01 Feb 2017 03:02:32 PM UTCamadeStatusNone=>Fixed
    Tue 31 Jan 2017 01:59:11 PM UTCNoneAttached File-=>Added bad_sequence, #39625

    Back to the top


    Powered by Savane 3.1-cleanup1