[screen-devel] Layout bug, canvas assert fails

[Michal Grochmal]

Hi,

Whilst debugging a small issue with the screen layout I believe that I
found quite a nasty bug.  If I use the following ~/.screenrc :

    layout new lay1
    split -v
    layout new lay2
    split -v
    layout attach lay2

And run:

    screen

My screen starts correctly.  But if I perform the following:

    screen -dm && screen -r

Or simply

    screen -dm
    screen -r

The layout is not attached to, instead I am presented with an empty
window (without a process running inside it).  Moreover, when I use `^A
: layout next` the screen process hangs and enters an infinite loop (I
suspect an infinite loop given the high CPU usage).  I need to kill that
process.

SPECULATION
-----------

On my system screen is SUID root ('cause i need multiuser) and, once I
kill the non-privileged user process, screen also hangs as a root
process.  This is not always replicable, and I apologise that I cannot
test this part properly, but it is a shared system where performing
this kind of DoS is not kind to other users.

DEBUG
-----

Speculation aside I've compiled screen with `-DDEBUG` and did the tests
above.  Using that same .screenrc and performing:

    ./screen -dm

I get the following debug output (the relevant parts):

    AclSetPerm(uu, user 'grochmal', mode '+a', object '#?')
    Serversocket owned by 4172
    findrcfile: you specified '/usr/etc/screenrc'
    secfopen(/usr/etc/screenrc, r)
    StartRc: '/usr/etc/screenrc' no good. ignored
    findrcfile: you specified nothing...
      ...nothing in $SCREENRC, defaulting $HOME/.screenrc
    secfopen(/home/grochmal/.screenrc, r)

    (...)

    Parse 2048 layout new lay1

    - new arg layout new lay1

    - arg done, 'layout' rest new lay1

    - new arg new lay1

    - arg done, 'new' rest lay1

    - new arg lay1

    - arg done, 'lay1' rest
    -- layout
    -- new
    -- lay1
    RcLine: WARNING, no display no user! Session owner executes command
    Activate(-1)
    Parse 2048 split -v

    - new arg split -v

    - arg done, 'split' rest -v

    - new arg -v

    - arg done, '-v' rest
    -- split
    -- -v
    RcLine: WARNING, no display no user! Session owner executes command
    Msg('/home/grochmal/.screenrc: split: display required') (0);
    Parse 2048 layout new lay2

    - new arg layout new lay2

    - arg done, 'layout' rest new lay2

    - new arg new lay2

    - arg done, 'new' rest lay2

    - new arg lay2

    - arg done, 'lay2' rest
    -- layout
    -- new
    -- lay2
    RcLine: WARNING, no display no user! Session owner executes command
    Activate(-1)
    Parse 2048 split -v

    - new arg split -v

    - arg done, 'split' rest -v

    - new arg -v

    - arg done, '-v' rest
    -- split
    -- -v
    RcLine: WARNING, no display no user! Session owner executes command
    Msg('/home/grochmal/.screenrc: split: display required') (0);
    Parse 2048 #layout dump ~/layout-dump

    Parse 2048 layout attach lay2


    - new arg layout attach lay2

    - arg done, 'layout' rest attach lay2

    - new arg attach lay2

    - arg done, 'attach' rest lay2

    - new arg lay2

    - arg done, 'lay2' rest
    -- layout
    -- attach
    -- lay2
    RcLine: WARNING, no display no user! Session owner executes command
    Parse 2048

    UID 861213  EUID 861213
    We open one default window, as screenrc did not specify one.
    NewWindow: StartAt -1
    NewWindow: aka     NULL
    NewWindow: dir     NULL
    NewWindow: term    NULL
    NWin: aka     NULL
    NWin: wlock   0
    NWin: Lflag   0
    Makewin creating 0
    fcntl(5, F_SETFL, FNBLOCK)

    (...)

    ResizeDisplay: to (126,38).
    ResizeDisplay: No change
    ChangeScrollRegion: (0 - 37)
    Flush(): 82
    Clear 0,0 0-125 125,37 uselayfn=0 bce=0
    SetColor 9 9 -> 9 9
    (0 0 -> 0 0)
    we don't want to adapt all our windows to the display
    CheckScreenSize: screen is (126,38)
    CheckScreenSize: No change -> return.
    RemoveLoginSlot: removing your logintty
    RemoveLoginSlot: utmpok == 0
     slot 0 zapped
    couln't zap slot -> do mesg n
    SetUtmp 0 will get slot 39624461...
    rlogin hostname: ':pts/0:S.0'
    SetUtmp successful
    KillLayerChain 0x25efed0
    Deq event fd 0 type 0 queued 0
    MakeDefaultCanvas 0,0 125,36
    ResizeCanvas: 0,0 125,36
    ASSERT(l->l_cvlist != cv) failed file canvas.c line 294

So yeah, we get a couple of warnings about the fact that there is no
display user, but since they're warnings screen continues.  Then when we
get to the canvas itself an ASSERT kills the process.

In the `-DDEBUG` binary `screen -dm` never starts.  In the binary that
is compiled without `-DDEBUG` the execution continues and the weird
(possibly dangerous?) behaviour happens.

SYSTEM INFORMATION
------------------

All the above has been performed on Arch Linux uname:

    Linux haps 4.9.8-1-ARCH #1 SMP PREEMPT \
        Mon Feb 6 12:59:40 CET 2017 x86_64 GNU/Linux

And on screen 4.5.0, both the non-debug and debug binary.

The reason I investigated is a question on stack overflow where a user
had the issue on Ubuntu with screen 4.3.0 :

http://unix.stackexchange.com/questions/344522/screen-layout-operations-in-screenrc-not-working

I believe it is a bug worth looking at.

I am not familiar with the screen codebase but might be able to write a
patch, although it would take me a considerable amount of time.  Am not
sure how many development resources are available to fix bugs on
screen-devel, so, if this is really a bug (i.e. not something I'm doing
wrong) and the time it would take me is not of huge importance I might
chip in.

Thanks,
--
Mike Grochmal
GPG key ID 0xC840C4F6