[screen-devel] [PATCH 0/6] Fixes for several defects and warnings found by static analysis

[Václav Doležal]

Hello,
I'm sending several patches addressing issues found by static analysis.

Regards,
Václav Doležal

--

Defects found:
Error: BUFFER_SIZE (CWE-120):
screen-4.6.2/screen.c:1274: buffer_size: Calling strncpy with a source string 
whose length (6 chars) is greater than or equal to the size argument (6) will 
fail to null-terminate "ap".
# 1272|     while (ap >= av0) {
# 1273|       if (!strncmp("screen", ap, 6)) {
# 1274|->       strncpy(ap, "SCREEN", 6); /* name this process "SCREEN-BACKEND" 
*/
# 1275|         break;
# 1276|       }
Note: this is for replacing "screen" with "SCREEN" - I think omitting 
terminating \0 is intentional -> memcpy(3) should be used

Error: RESOURCE_LEAK (CWE-772):
screen-4.6.2/socket.c:723: leaked_handle: Handle variable "s" going out of 
scope leaks the handle.
#  721|       {
#  722|         Msg(errno, "getcwd");
#  723|->       return;
#  724|       }
#  725|     if (nwin->term != nwin_undef.term)

Error: BUFFER_SIZE_WARNING (CWE-120):
screen-4.6.2/pty.c:282: buffer_size_warning: Calling strncpy with a maximum 
size argument of 32 bytes on destination array "TtyName" of size 32 bytes might 
leave the destination string unterminated.
#  280|       }
#  281|     signal(SIGCHLD, sigcld);
#  282|->   strncpy(TtyName, m, sizeof(TtyName));
#  283|     initmaster(f);
#  284|     *ttyn = TtyName;

Error: USE_AFTER_FREE (CWE-825):
screen-4.6.2/resize.c:950: freed_arg: "free" frees "nmlines".
screen-4.6.2/resize.c:959: double_free: Calling "free" frees pointer "nmlines" 
which has already been freed.
screen-4.6.2/resize.c:953: freed_arg: "free" frees "nhlines".
screen-4.6.2/resize.c:961: double_free: Calling "free" frees pointer "nhlines" 
which has already been freed.
#  957|               Msg(0, "%s", strnomem);
#  958|               if (nmlines)
#  959|->               free(nmlines);
#  960|               if (nhlines)
#  961|->               free(nhlines);
#  962|               return -1;
#  963|             }
Note: Introduced in ff98d7ff5847e07a55b0c40c2ccc3bc430226ca0

Several warnings about misleading indentation.

--

Vaclav Dolezal (6):
  Use memcpy(3) in string substitution
  Fix file descriptor leak
  Revert "those 0 assignment made rest of code totally not working"
  Fix for nomem handling in resize.c:ChangeWindowSize()
  Fix possible unterminated string
  Fix confusing indentation on several places

 src/fileio.c |  2 +-
 src/help.c   |  6 +++---
 src/pty.c    |  8 ++++++-
 src/resize.c | 45 ++++++++++++++++++--------------------
 src/screen.c | 70 ++++++++++++++++++++++++++++++------------------------------
 src/socket.c |  4 +++-
 6 files changed, 70 insertions(+), 65 deletions(-)

-- 
2.14.5