0088-pmap: Always check the return value of fgets().

Otherwise "the contents of the array remain unchanged and a null pointer
is returned" or "the array contents are indeterminate and a null pointer
is returned".

Signed-off-by: Craig Small <csmall@enc.com.au>

0087-pmap: Fix parsing error in config_read().

$ echo '[' > crash
$ pmap -C crash $$
Segmentation fault (core dumped)

Signed-off-by: Craig Small <csmall@enc.com.au>

0086-pmap: Prevent integer overflow in main().

Unlikely to ever happen, but just in case.

Signed-off-by: Craig Small <csmall@enc.com.au>

0085-pmap.c: Plug memory leak in range_arguments().

Also, simplify the code slightly (but functionally equivalent). Check
the return value of xstrdup() only once (yes, it can return NULL).

Adapted slightly to remove goto and leave the format of checks the same.
A lot of the fixes were already in newlib, caught by coverity

References:
 commit 25f655891f4016ff9e241f1242e995d35e6b554c

Signed-off-by: Craig Small <csmall@enc.com.au>

0027-skill: Prevent multiple overflows in ENLIST().

First problem: saved_argc was used to calculate the size of the array,
but saved_argc was never initialized. This triggers an immediate heap-
based buffer overflow:

$ skill -c0 -c0 -c0 -c0
Segmentation fault (core dumped)

Second problem: saved_argc was not the upper bound anyway, because one
argument can ENLIST() several times (for example, in parse_namespaces())
and overflow the array as well.

Third problem: integer overflow of the size of the array.

0026-skill: Fix double-increment of pid_count.

No need to "pid_count++;" because "ENLIST(pid," does it already. Right
now this can trigger a heap-based buffer overflow.

Also, remove the unneeded "pid_count = 0;" (it is static, and
skillsnice_parse() is called only once; and the other *_count variables
are not initialized explicitly either).

0024-skill: Always NULL-terminate argv.

The memmove() itself does not move the NULL-terminator, because nargs is
decremented first. Copy how skill_sig_option() does it: decrement nargs
last, and remove the "if (nargs - i)" (we are in "while (i < nargs)").

0023-skill: Fix getline() usage.

man getline: "If *lineptr is set to NULL and *n is set 0 before the
call, then getline() will allocate a buffer for storing the line. This
buffer should be freed by the user program even if getline() failed."

0022-skill: Simplify the kill_main() loop.

Right now the "loop=0; break;" is never reached.

0021-pwdx: Fix a misleading comment.

It sounds like an off-by-one, but the code itself is correct.

0020-pidof: Prevent integer overflows with grow_size().

Note: unlike "size" and "omit_size", "path_alloc_size" is not multiplied
by "sizeof(struct el)" but the checks in grow_size() allow for a roughly
100MB path_alloc_size, which should be more than enough for readlink().

0019-pidof: Do not memleak pidof_root if multiple -c options.

0018-pidof: Do not skip the NULL terminator in cmdline.

This should never happen (cmdline[0] should always be non-NULL), but
just in case.

0017-pidof: Get the arg1 base name with get_basename().

Same as program_base, cmd_arg0base, and exe_link_base.

0015-tload: Prevent integer overflows of ncols, nrows, and scr_size.

Also, use xerrx() instead of xerr() since errno is not set.

0014-tload: Prevent a buffer overflow when row equals nrows.

When max_scale is very small, scale_fact is very small, row is equal to
nrows, p points outside screen, and the write to *p is out-of-bounds.

0013-tload: Use snprintf() instead of sprintf().

0012-tload: Call longjmp() 1 instead of 0.

Do it explicitly instead of the implicit "longjmp() cannot cause 0 to be
returned. If longjmp() is invoked with a second argument of 0, 1 will be
returned instead."

0011-tload: Use standard names instead of numbers.

0009-uptime: Check the return value of various functions.

0007-pgrep: Always null-terminate the cmd*[] buffers.

Otherwise, man strncpy: "If there is no null byte among the first n
bytes of src, the string placed in dest will not be null-terminated."

0006-pgrep: Initialize the cmd*[] stack buffers.

Otherwise (for example), if the (undocumented) opt_echo is set, but not
opt_long, and not opt_longlong, and not opt_pattern, there is a call to
xstrdup(cmdoutput) but cmdoutput was never initialized:

sleep 60 & echo "$!" > pidfile
env -i LD_DEBUG=`perl -e 'print "A" x 131000'` pkill -e -c -F pidfile | xxd
...
000001c0: 4141 4141 4141 4141 4141 4141 4141 4141  AAAAAAAAAAAAAAAA
000001d0: 4141 4141 4141 4141 fcd4 e6bd e47f 206b  AAAAAAAA...... k
000001e0: 696c 6c65 6420 2870 6964 2031 3230 3931  illed (pid 12091
000001f0: 290a 310a                                ).1.
[1]+  Terminated              sleep 60

(the LD_DEBUG is just a trick to fill the initial stack with non-null
bytes, to show that there is uninitialized data from the stack in the
output; here, an address "fcd4 e6bd e47f")

0005-pgrep: Simplify the match_*() functions.

0004-pgrep: Replace buf+1 with buf in read_pidfile().

Unless we missed something, this makes it unnecessarily difficult to
read/audit.

0003-pgrep: Replace ints with longs in strict_atol().

atol() means long, and value points to a long.

0002-pgrep: Prevent integer overflow of list size.

Not exploitable (not under an attacker's control), but still a potential
non-security problem. Copied, fixed, and used the grow_size() macro from
pidof.c.

Signed-off-by: Craig Small <csmall@enc.com.au>

top: a tweak to the recent 256-color termninal support

We now use the actual terminfo 'max_colors' value with
the 'color mapping' screen, not that hard coded '256'.

Reference(s):
https://gitlab.com/procps-ng/procps/issues/96

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: treat all of those vertical scroll keys uniformly

When not displaying all tasks (the 'i' toggle is off),
the concept of vertical scrolling has no real meaning.

However, only 2 keys (up/down) impacting that vertical
position were currently being disabled with this mode.

This patch will extend such treatment to the following
additional vertical impact keys: pgup,pgdn,home & end.

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: force return to row 1 for thread mode transitions

This program does a good job of policing that vertical
scrolled position, ensuring that total tasks are never
exceeded. However, during transitions from thread mode
to normal task mode (the 'H' toggle) that wasn't true.

And while there was no real harm done, it did make the
use of up/down arrow keys "appear" disabled especially
if that scroll message was not displayed ('C' toggle).

This patch simply forces a return to row #1 whenever a
user toggles that display between thread & task modes.

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: fix 'iokey()' flaw preventing proper translations

As it turns out, the very first entry in the 'iokey()'
tinfo_tab was preventing the proper translation of the
simulated PgUp/PgDn keys (ctrl+meta+k/j). Ignoring the
tortured history behind the most recent change to that
entry, this patch restores the previous value and once
again properly translates these particular keystrokes.

Signed-off-by: Jim Warner <james.warner@comcast.net>

0067-ps/sortformat.c: Avoid "sep_loc + 1" when sep_loc is NULL.

0066-ps/sortformat.c: Handle large width in aix_format_parse().

Unlikely to ever happen, since it would imply a very large string, but
better safe than sorry.

---------------------------- adapted for newlib branch
. now uses 'xmalloc' vs. unchecked stdlib 'malloc'
. the member 'need' was removed from 'format_node'

Signed-off-by: Jim Warner <james.warner@comcast.net>

0065-ps/sortformat.c: Catch negative width in format_parse().

The existing strspn() check guarantees that the string contains no '-'
but atoi() does not catch errors, especially not integer overflows.

0064-ps/sortformat.c: Double-check chars in verify_short_sort().

To avoid an out-of-bounds access at checkoff[tmp]. The strspn() at the
beginning of the function protects against it already, but double-check
this in case of some future change.

0062-ps/display.c: Always exit from signal_handler().

Right now, "we _exit() anyway" is not always true: for example, the
default action for SIGURG is to ignore the signal, which means that
"kill(getpid(), signo);" does not terminate the process. Call _exit()
explicitly, in this case (rather than exit(), because the terminating
kill() calls do not call the functions registered with atexit() either).

0061-ps/output.c: Always null-terminate outbuf in show_one_proc().

Before "strlen(outbuf)", if one of the pr_*() functions forgot to do it.
This prevents an out-of-bounds read in strlen(), and an out-of-bounds
write in "outbuf[sz] = '\n'". Another solution would be to replace
strlen() with strnlen(), but this is not used anywhere else in the
code-base and may not exist in all libc's.

---------------------------- adapted for newlib branch
. adapted via 'patch' without rejections

Signed-off-by: Jim Warner <james.warner@comcast.net>

0060-ps/output.c: Protect outbuf in various pr_*() functions.

pr_bsdstart(): Replace "strcpy(outbuf," with "snprintf(outbuf, COLWID,"
(which is used in all surrounding functions). (side note: the fact that
many pr_*() functions simply return "snprintf(outbuf, COLWID," justifies
the "amount" checks added to show_one_proc() by the "ps/output.c:
Replace strcpy() with snprintf() in show_one_proc()." patch)

pr_stime(): Check the return value of strftime() (in case of an error,
"the contents of the array are undefined").

help_pr_sig(): Handle the "len < 8" case, otherwise "sig+len-8" may
point outside the sig string.

pr_context(): Handle the empty string case, or else "outbuf[len-1]"
points outside outbuf.

---------------------------- adapted for newlib branch
. logic is quite different with 'stacks' vs. 'proc_t'

Signed-off-by: Jim Warner <james.warner@comcast.net>

0059-ps/output.c: Enforce a safe range for max_rightward.

Enforce a maximum max_rightward of OUTBUF_SIZE-1, because it is used in
constructs such as "snprintf(outbuf, max_rightward+1," (we could remove
the extra check at the beginning of forest_helper() now, but we decided
to leave it, as a precaution and reminder).

The minimum max_rightward check is not strictly needed, because it is
unsigned. However, we decided to add it anyway:

- most of the other variables are signed;

- make it visually clear that this case is properly handled;

- ideally, the minimum max_rightward should be 1, not 0 (to prevent
  integer overflows such as "max_rightward-1"), but this might change
  the behavior/output of ps, so we decided against it, for now.

Instead, we fixed the only function that overflows if max_rightward is
0. Also, enforce the same safe range for max_leftward, although it is
never used throughout the code-base.

---------------------------- adapted for newlib branch
. adapted via 'patch' without rejections

Signed-off-by: Jim Warner <james.warner@comcast.net>

0058-ps/output.c: Replace strcpy() with snprintf() in show_one_proc().

This strcpy() should normally not overflow outbuf, but names can be
overridden (via -o). Also, check "amount" in all cases.

---------------------------- adapted for newlib branch
. we don't use that 'likely/unlikely' crap in newlib

Signed-off-by: Jim Warner <james.warner@comcast.net>

0057-ps/output.c: Remove the page_shift variable.

It is static and not used anywhere.

---------------------------- adapted for newlib branch
. limited to whitespace/formatting differences

Signed-off-by: Jim Warner <james.warner@comcast.net>

0056-ps/output.c: Check return value of mmap() in init_output().

We decided not to check the return value of the mprotect() calls,
because they are not vital to the operation of ps.

---------------------------- adapted for newlib branch
. many formatting/whitespace differences

Signed-off-by: Jim Warner <james.warner@comcast.net>

0055-ps/display.c: Harden show_tree().

1/ Do not go deeper than the size of forest_prefix[], to prevent a
buffer overflow (sizeof(forest_prefix) is roughly 128K, but the maximum
/proc/sys/kernel/pid_max is 4M). (actually, we go deeper, but we stop
adding bytes to forest_prefix[])

2/ Always null-terminate forest_prefix[] at the current level.

---------------------------- adapted for newlib branch
. logic is quite different with 'stacks' vs. 'proc_t'
. a commented out 'debug' line was no longer present

Signed-off-by: Jim Warner <james.warner@comcast.net>

0054-ps/output.c: Fix outbuf overflows in pr_args() etc.

Because there is usually less than OUTBUF_SIZE available at endp.

---------------------------- adapted for newlib branch
. logic is quite different with 'stacks' vs. 'proc_t'
. ps no longer deals with the library 'FILL...' flags

Signed-off-by: Jim Warner <james.warner@comcast.net>

0053-ps/output.c: Harden forest_helper().

This patch solves several problems:

1/ Limit the number of characters written (to outbuf) to OUTBUF_SIZE-1
(-1 for the null-terminator).

2/ Always null-terminate outbuf at q.

3/ Move the "rightward" checks *before* the strcpy() calls.

4/ Avoid an integer overflow in these checks (e.g., rightward-4).

0052-ps/output.c: Handle negative snprintf() return value.

May happen if strlen(src) > INT_MAX for example. This patch prevents
escaped_copy() from increasing maxroom and returning -1 (= number of
bytes consumed in dst).

---------------------------- adapted for newlib branch
. formerly applied to proc/escape.c
. function was moved to ps/output.c

Signed-off-by: Jim Warner <james.warner@comcast.net>

0048-ps/output.c: Make sure all escape*() arguments are safe.

The SECURE_ESCAPE_ARGS() macro solves several potential problems
(although we found no problematic calls to the escape*() functions in
procps's code-base, but had to thoroughly review every call; and this is
library code):

1/ off-by-one overflows if the size of the destination buffer is 0;

2/ buffer overflows if this size (or "maxroom") is negative;

3/ integer overflows (for example, "*maxcells+1");

4/ always null-terminate the destination buffer (unless its size is 0).

---------------------------- adapted for newlib branch
. formerly applied to proc/escape.c
. function was moved to ps/output.c

Signed-off-by: Jim Warner <james.warner@comcast.net>

ps: move other initialization code after setREL macros

While the previous patch concerned an essential change
to avoid dereferencing those NULL pointers, this patch
could be considered optional. For consistency, it just
puts all initialization logic after the setREL macros.

[ plus along the way some inter-function spacing was ]
[ standardized with just a single blank line between ]

Signed-off-by: Jim Warner <james.warner@comcast.net>

ps: ensure functions not called prior to setREL macros

Under newlib design, ps must loop though all potential
print functions so as to gather the appropriate enum's
while establishing the 'relative' equivalent. The keys
to the setREL/chkREL macros are a NULL 'outbuf' param.

It's imperative that no other functions be called with
that NULL value. Unfortunately, several instances were
found where this was violated. They are now corrected!

Signed-off-by: Jim Warner <james.warner@comcast.net>

ps/output.c: eliminate one irritating compiler warning

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: provide the means to exploit a 256-color terminal

With the Qualys security audit, we began to harden our
treatment of the top rcfile. In particular, the values
read were checked so as to prevent some malicious user
from editing it in order to achieve an evil objective.

However when it came to colors I was surprised to find
that at least one user edited the rcfile for 256-color
support. Unfortunately, our new checks prevented this.

So this commit will provide the means to exploit those
extra colors with no need to manually edit the rcfile.

Reference(s):
https://gitlab.com/procps-ng/procps/issues/96

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: sanitized some potentially corrupt 'Inspect' data

This guards against rcfile 'Inspect' entries which may
include non-printable characters. While this shouldn't
occur, we have no real control over those crazy users.

[ and, while such data can't be used maliciously, it ]
[ does adversely impact such a user's screen display ]

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: prevent buffer overruns in 'inspection_utility()'

For our master branch, a Qualys patch referenced below
was reverted as being unwarranted. That original patch
was not applied in this branch so there was no revert.

However, there was 1 specific problem their patch had,
in fact, prevented. Thus, this patch now addresses it.

Reference(s):
. original qualys patch
0109-top-Protect-scat-from-buffer-overflows.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: add another field sanity check in 'config_file()'

Until the Qualys security audit I had never considered
it a possibility that some malicious person might edit
the top config file to achieve some nefarious results.

And while the Qualys approach tended to concentrate on
the symptoms from such an effort, subsequent revisions
more properly concentrated on startup and that rcfile.

This commit completes those efforts with 1 more field.

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: don't mess with groff line length in man document

I've long since forgotten why the attempt to influence
groff line lengths was made. However, I did receive an
email regarding problems formatting postscript output.

Hopefully this patch will eliminate any such problems.

Signed-off-by: Jim Warner <james.warner@comcast.net>

library: avoid problems involving 'supgid' mishandling

Following that patch referenced below, the top SUPGRPS
field would produce a segmentation fault and ps SUPGRP
would often show "(null)". Such problems resulted from
some faulty logic in the status2proc() routine dealing
with 'Groups' (supgid) which served as a source field.

For many processes the original code produced an empty
string which prevented conversion to the expected "-".
Moreover, prior to release 3.3.15 such an empty string
will become 0 after strtol() which pwcache_get_group()
translates to 'root' yielding very misleading results.

So, now we'll check for empty '/proc/#/status/Groups:'
fields & consistently provide a "-" value for callers.

[ we'll also protect against future problems in that ]
[ new qualys logic by always ensuring valid 'supgrp' ]
[ pointers - logic which revealed our original flaw! ]

Reference(s):
. original qualys patch
0071-proc-readproc.c-Harden-supgrps_from_supgids.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

library: refactor #define FALSE_THREADS dependent code

This refactor was done in response to the Qualys patch
referenced below, which deals with some 'readeither()'
flaws under the master branch. Under our newlib branch
those flaws mostly disappear since the function is now
private. But without a redesign the #define is broken.

When the #define FALSE_THREADS is active, some special
strings showing "[ duplicate ENUM ]" will appear under
each child thread. Note that the real reason for those
appearing isn't being exercised, only their mechanics.

In reality, they only show when a user duplicates such
enums in a results stack & only 1 instance can own it.

Reference(s):
. original qualys patch
0084-proc-readproc.c-Work-around-a-design-flaw-in-readeit.patch
. QUICK_THREADS became FALSE_THREADS
commit c546d9dd4409ee11cd466c99a820a3b5dadfe3f4

Signed-off-by: Jim Warner <james.warner@comcast.net>

library: clean up some miscellaneous compiler warnings

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: just respond to the increased command name length

The command name for running tasks is displayed by top
in a variable length field, so the increase from 16 to
64 bytes was not a problem. However, there's one place
where top is sensitive to length - insp_view_choice().

So, this patch just bumps a buffer used to display it.

Reference(s):
. master branch increase to 64
commit 2cfdbbe897f0d4e41460c7c2b92acfc5804652c8

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: eliminate a couple of warnings of -Wunused-result

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: ensure sane rcfile values for the remaining stuff

This will protect some remaining rcfile variables from
a possible manual editing of top's configuration file.

[ and correct two #error related boo-boos introduced ]
[ with the system default rcfile in the commit shown ]

Reference(s):
. introduced /etc/topdefaultrc
commit 55a42ae040eaa19fd3089f56d98ccbde5a9abc3a

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: Prevent out-of-bounds writes in PUFF(). __Tweaked

This commit moves some overhead to the Batch mode path
where it's needed. And given the new 'else if' test we
can delete some now redundant logic in the other path.

Reference(s):
. original qualys patch
0117-top-Prevent-out-of-bounds-writes-in-PUFF.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: address 'show_special()' o-o-b read/write concern

This patch addresses a potential (but unlikely) buffer
overflow by reducing, if necessary, a memcpy length by
3 bytes to provide for an eol '\0' and 2 unused buffer
positions which also might receive the '\0' character.

[ note to future analysis tool: just because you see ]
[ binary data being manipulated in the routine, that ]
[ doesn't mean such function was passed binary data! ]

Reference(s):
. original qualys patch
0116-top-Fix-out-of-bounds-read-write-in-show_special.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: prevent buffer overflow potential in all routines

Whereas an original patch (referenced below) addressed
some symptoms related to manually edited config files,
this solution deals with root causes. And it goes much
beyond any single top field by protecting all of top's
fields. Henceforth, a duplicated field is not allowed.

Reference(s):
. original qualys patch
0114-top-Prevent-buffer-overflow-in-calibrate_fields.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: check sortindx risk exposure (not treat symptoms)

Rather than validate the window's 'sortindx' each time
it was referenced (as was done in the patch below), we
now ensure the validity just once when the config file
is read. Thereafter, a running top will police bounds.

Reference(s):
. original qualys patch
0102-top-Check-sortindx.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: other graph_cpus, graph_mems, and summ_mscale fix

This patch replaces an original patch referenced below
(omitted under this branch). We now validate variables
'graph_cpus', 'graph_mems' and 'summ_mscale' just once
at startup. Thereafter, top enforces the proper range.

[ we afford the same treatment to that 'task_mscale' ]
[ variable, which was ignored in the original patch. ]

Reference(s):
. original qualys patch
0099-top-Check-graph_cpus-graph_mems-and-summ_mscale.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: Do not default to the cwd in configs_r... Tweaked

While it's only documented (so far) in commit text and
an occasional email I've tried to maintain some coding
standards primarily for reference/navigation purposes.
They also served, I felt, as useful mental challenges.

Someday I will get around to formerly documenting them
but in the meantime here are the ones for this commit:

. functions are grouped into logical (i hope) sections
. functions & sections are ordered to avoid prototypes
. function names are alphabetical within every section
. all functions & sections must be referenced in top.h

This patch just attempts to honor the above standards,
while also covering this new behavior in the man page.

[ please note that the net result of these 2 patches ]
[ is simply to avoid pathname truncations should our ]
[ limit of 1024 be exceeded. they do not have a role ]
[ in solving the 'local privilege escalation' issue. ]

[ and we can never prevent a user from setting their ]
[ HOME var to a directory writable by some attacker! ]

[ the only real protection for that CVE-2018-1122 is ]
[ those soon to be enhanced rcfile integrity checks, ]
[ achieved through several of the following patches. ]

Reference(s):
. original qualys patch
0097-top-Do-not-default-to-the-cwd-in-configs_read.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>

library: adapt for increased (cmd) program name length

In the new library 'cmd' is dynamically allocated just
like 'cmdline'. This will align us with the ref below.

Reference(s):
. master branch increase to 64
commit 2cfdbbe897f0d4e41460c7c2b92acfc5804652c8

Signed-off-by: Jim Warner <james.warner@comcast.net>

0117-top: Prevent out-of-bounds writes in PUFF().

This patch prevents three problems:

1/ Because snprintf() returns "the number of characters (excluding the
terminating null byte) which would have been written to the final string
if enough space had been available", _eol may point past the end of _str
and write out-of-bounds (in Batch mode).

2/ _eol is never checked against _str, so "while (*(--_eol) == ' ');"
may point _eol below _str and write out-of-bounds (in Batch mode).

3/ Sanity-check Pseudo_row to protect the strcpy().

0115-top: Harden calibrate_fields().

- Make sure i is at least 1 before "i - 1" and "--i".

- Initialize endpflg (to 0, as it was originally, since it is static)
  before the "for" loop (the "break" may leave endpflg uninitialized,
  for example).

0113-top: Impose a minimum on Screen_cols.

The safety of the critical function task_show() depends on the sanity of
Screen_cols. Just copy the tests on w_cols to Screen_cols (from the same
function adj_geometry()).

0112-top: Prevent integer overflow in adj_geometry().

0111-top: Limit Width_mode to SCREENMAX.

adj_geometry() limits to SCREENMAX too, but belt and suspenders, and
might as well tell the user about it.

0110-top: Prevent integer overflows in config_file() and other_selection().

0108-top: Always exit from sig_abexit().

The default action for SIGURG is to ignore the signal, for example.
This is very similar to the patch "ps/display.c: Always exit from
signal_handler()."

0107-top: Initialize struct sigaction in before().

0106-top: Fix snprintf() call in capsmk().

Replace "snprintf(msg, sizeof(pmt)" with "snprintf(msg, sizeof(msg)".
Luckily sizeof(pmt) == sizeof(msg), but fix it anyway.

0104-top: Initialize cp in task_show().

Found no problematic case at the moment, but this is a cheap
just-in-case.

0103-top: Protect macro parameters.

---------------------------- adapted for newlib branch
. the 'isBUSY' macro is quite different under newlib

Signed-off-by: Jim Warner <james.warner@comcast.net>

0101-top: Check width and col.

Otherwise they may lead to out-of-bounds writes (snprintf() returns the
number of characters which would have been written if enough space had
been available).

Also, make sure buf is null-terminated after COLPLUSCH has been written.

0100-top: Check Rc.fixed_widest.

Otherwise it leads to crashes (for example, setting it to 2147483600 in
the configuration file segfaults top).

0098-top: Check i when setting Curwin in config_file().

Otherwise it leads to out-of-bounds reads (and maybe writes).

0097-top: Do not default to the cwd in configs_read().

If the HOME environment variable is not set, or not absolute, use the
home directory returned by getpwuid(getuid()), if set and absolute
(instead of the cwd "."); otherwise, set p_home to NULL.

To keep the changes to a minimum, we rely on POSIX, which requires that
fopen() fails with ENOENT if the pathname (Rc_name) is an empty string.
This integrates well into the existing code, and makes write_rcfile()
work without a change.

Also, it makes the code in configs_read() easier to follow: only set and
use p_home if safe, and only set Rc_name if safe (in all the other cases
it is the empty string, and the fopen() calls fail). Plus, check for
snprintf() truncation (and if it happens, reset Rc_name to the empty
string).

Important note: top.1 should probably be updated, since it mentions the
fallback to the current working directory.

0096-top: Fix double-fclose() in configs_read().

It happens only if RCFILE_NOERR is defined (it is not, by default).

0084-proc/readproc.c: Work around a design flaw in readeither().

readeither() caches (in new_p) a pointer to the proc_t of a task-group
leader, but readeither()'s callers can do pretty much anything with the
proc_t structure passed to and/or returned by this function. For
example, they can 1/ free it or 2/ recycle it (by passing it to
readeither() as x).

1/ leads to a use-after-free, and 2/ leads to unexpected behavior when
taskreader()/simple_readtask() is called with new_p equal to x (this is
not a theoretical flaw: 2/ happens in readproctab3() when want_task()
returns false and p is a group leader).

As a workaround, we keep a copy of new_p's first member (tid) in static
storage, and the next times we enter readeither() we check this "canary"
against the tid in new_p: if they differ, we reset new_p to NULL, which
forces the allocation of a new proc_t (the new "leader", or reference).

This always detects 2/ (because free_acquired(x,1) memsets x and hence
new_p); always detects 1/ if freed via free_acquired() and/or freeproc()
(very likely, otherwise memory may be leaked); probably detects 1/ even
if freed directly via free() (because the canary is the first member of
proc_t, likely to be overwritten by free()); but can not detect 1/ if
free() does not write to new_p's chunk at all.

Moreover, accessing new_p->tid to check the canary in case 1/ is itself
a use-after-free, so a better long-term solution should be implemented
at some point (we wanted to avoid intrusive and backward-incompatible
changes in this library function, hence this imperfect workaround).

---------------------------- adapted for newlib branch
. adapted via 'patch' (rejected due to 'xcalloc' ref)
. with loss of both readproctab functions, most no longer true

Signed-off-by: Jim Warner <james.warner@comcast.net>

0082-proc/readproc.c: Fix double-free()s in readtask().

If QUICK_THREADS is not defined (it is not by default, but most
distributions enable it) and task_dir_missing is true (only on very old
kernels), then readtask() forgets to reset some of the struct proc_t t's
members, which later results in double-free()s in free_acquired().

For now, we simply synchronized the list of members to be reset with the
list of members freed in free_acquired().

---------------------------- adapted for newlib branch
. now 'cmd' is also dynamic
. just synchronized with those freed in free_acquired
. QUICK_THREADS is now FALSE_THREADS, serving different purpose
. entire patch will be effectively reverted with upcoming refactor

Signed-off-by: Jim Warner <james.warner@comcast.net>

0080-proc/readproc.c: Harden openproc().

Replace xmalloc() with xcalloc().

---------------------------- adapted for newlib branch
. trade xcalloc() for calloc()
. thus we must account for potential ENOMEM

Signed-off-by: Jim Warner <james.warner@comcast.net>

0078-proc/readproc.c: Harden simple_nextpid().

Replace memcpy+strcpy with snprintf.

---------------------------- adapted for newlib branch
. adapted via 'patch' (without rejections)

Signed-off-by: Jim Warner <james.warner@comcast.net>

0077-proc/readproc.c: Harden fill_cgroup_cvt().

Check the return value of snprintf(), otherwise dst may point
out-of-bounds when it reaches the end of the dst_buffer (the snprintf()
always returns 1 in that case, even if there is not enough space left),
and vMAX becomes negative and is passed to snprintf() as a size_t.

---------------------------- adapted for newlib branch
. adapted via 'patch (without rejections)

Signed-off-by: Jim Warner <james.warner@comcast.net>

0076-proc/readproc.c: Harden vectorize_this_str().

This detects an integer overflow of "strlen + 1", prevents an integer
overflow of "tot + adj + (2 * pSZ)", and avoids calling snprintf with a
string longer than INT_MAX. Truncate rather than fail, since the callers
do not expect a failure of this function.

---------------------------- adapted for newlib branch
. logic is now in pids.c
. former 'vectorize_this_str' is now 'pids_vectorize_this'

Signed-off-by: Jim Warner <james.warner@comcast.net>

0075-proc/readproc.c: Harden read_unvectored().

1/ Prevent an out-of-bounds write if sz is 0.

2/ Limit sz to INT_MAX, because the return value is an int, not an
unsigned int (and because if INT_MAX is equal to SSIZE_MAX, man 2 read
says "If count is greater than SSIZE_MAX, the result is unspecified.")

3/ Always null-terminate dst (unless sz is 0), because a return value of
0 because of an open() error (for example) is indistinguishable from a
return value of 0 because of an empty file.

4/ Use an unsigned int for i (just like n), not an int.

5/ Check for snprintf() truncation.

---------------------------- adapted for newlib branch
. adapted via 'patch (without rejections)

Signed-off-by: Jim Warner <james.warner@comcast.net>

0074-proc/readproc.c: Fix bugs and overflows in file2strvec().

Note: this is by far the most important and complex patch of the whole
series, please review it carefully; thank you very much!

For this patch, we decided to keep the original function's design and
skeleton, to avoid regressions and behavior changes, while fixing the
various bugs and overflows. And like the "Harden file2str()" patch, this
patch does not fail when about to overflow, but truncates instead: there
is information available about this process, so return it to the caller;
also, we used INT_MAX as a limit, but a lower limit could be used.

The easy changes:

- Replace sprintf() with snprintf() (and check for truncation).

- Replace "if (n == 0 && rbuf == 0)" with "if (n <= 0 && tot <= 0)" and
  do break instead of return: it simplifies the code (only one place to
  handle errors), and also guarantees that in the while loop either n or
  tot is > 0 (or both), even if n is reset to 0 when about to overflow.

- Remove the "if (n < 0)" block in the while loop: it is (and was) dead
  code, since we enter the while loop only if n >= 0.

- Rewrite the missing-null-terminator detection: in the original
  function, if the size of the file is a multiple of 2047, a null-
  terminator is appended even if the file is already null-terminated.

- Replace "if (n <= 0 && !end_of_file)" with "if (n < 0 || tot <= 0)":
  originally, it was equivalent to "if (n < 0)", but we added "tot <= 0"
  to handle the first break of the while loop, and to guarantee that in
  the rest of the function tot is > 0.

- Double-force ("belt and suspenders") the null-termination of rbuf:
  this is (and was) essential to the correctness of the function.

- Replace the final "while" loop with a "for" loop that behaves just
  like the preceding "for" loop: in the original function, this would
  lead to unexpected results (for example, if rbuf is |\0|A|\0|, this
  would return the array {"",NULL} but should return {"","A",NULL}; and
  if rbuf is |A|\0|B| (should never happen because rbuf should be null-
  terminated), this would make room for two pointers in ret, but would
  write three pointers to ret).

The hard changes:

- Prevent the integer overflow of tot in the while loop, but unlike
  file2str(), file2strvec() cannot let tot grow until it almost reaches
  INT_MAX, because it needs more space for the pointers: this is why we
  introduced ARG_LEN, which also guarantees that we can add "align" and
  a few sizeof(char*)s to tot without overflowing.

- Prevent the integer overflow of "tot + c + align": when INT_MAX is
  (almost) reached, we write the maximal safe amount of pointers to ret
  (ARG_LEN guarantees that there is always space for *ret = rbuf and the
  NULL terminator).

---------------------------- adapted for newlib branch
. there were many formatting differences
. i introduced several myself (especially comments)
. stdlib 'realloc' used, not that home grown xrealloc
. stdlib 'realloc' required extra 'return NULL' statement

Signed-off-by: Jim Warner <james.warner@comcast.net>

0073-proc/readproc.c: Harden file2str().

1/ Replace sprintf() with snprintf() (and check for truncation).

2/ Prevent an integer overflow of ub->siz. The "tot_read--" is needed to
avoid an off-by-one overflow in "ub->buf[tot_read] = '\0'". It is safe
to decrement tot_read here, because we know that tot_read is equal to
ub->siz (and ub->siz is very large).

We believe that truncation is a better option than failure (implementing
failure instead should be as easy as replacing the "tot_read--" with
"tot_read = 0").

---------------------------- adapted for newlib branch
. no real changes, patch refused due to mem alloc & failure return

Signed-off-by: Jim Warner <james.warner@comcast.net>

0072-proc/readproc.c: Harden stat2proc().

1/ Use a "size_t num" instead of an "unsigned num" (also, do not store
the return value of sscanf() into num, it was unused anyway).

2/ Check the return value of strchr() and strrchr().

3/ Never jump over the terminating null byte with "S = tmp + 2".

---------------------------- adapted for newlib branch
. newlib doesn't use that 'unlikely' crap
. the cmd field is now also dynamic (like cmdline)
. thus we must account for potential ENOMEM

Signed-off-by: Jim Warner <james.warner@comcast.net>

0071-proc/readproc.c: Harden supgrps_from_supgids().

1/ Prevent an integer overflow of t.

2/ Avoid an infinite loop if s contains characters other than comma,
spaces, +, -, and digits.

3/ Handle all possible return values of snprintf().

---------------------------- adapted for newlib branch
. we can't use xrealloc(), so we use realloc() instead
. and must account for a mem failure via a return of 1

Signed-off-by: Jim Warner <james.warner@comcast.net>

0070-proc/readproc.c: Harden status2proc().

1/ Do not read past the terminating null byte when hashing the name.

2/ S[x] is used as an index, but S is "char *S" (signed) and hence may
index the array out-of-bounds. Bit-mask S[x] with 127 (the array has 128
entries).

3/ Use a size_t for j, not an int (strlen() returns a size_t).

Notes:

- These are (mostly) theoretical problems, because the contents of
  /proc/PID/status are (mostly) trusted.

- The "name" member of the status_table_struct has 8 bytes, and
  "RssShmem" occupies exactly 8 bytes, which means that "name" is not
  null-terminated. This is fine right now, because status2proc() uses
  memcmp(), not strcmp(), but it is worth mentioning.

---------------------------- adapted for newlib branch
. newlib doesn't use that 'unlikely' crap
. newlib also had a '#ifdef FALSE_THREADS'

Signed-off-by: Jim Warner <james.warner@comcast.net>

0069-proc/readproc.c: Fix the unhex() function.

This function is unused (SIGNAL_STRING is defined by default, and if it
is not, procps does not compile -- for example, there is no "outbuf" in
help_pr_sig()) but fix it anyway. There are two bugs:

- it accepts non-hexadecimal characters (anything >= 0x30);

- "(c - (c>0x57) ? 0x57 : 0x30)" is always equal to 0x57.

---------------------------- adapted for newlib branch
. newlib doesn't use that 'unlikely' crap

Signed-off-by: Jim Warner <james.warner@comcast.net>

0051-proc/escape.c: Prevent buffer overflows in escape_command().

This solves several problems:

1/ outbuf[1] was written to, but not outbuf[0], which was left
uninitialized (well, SECURE_ESCAPE_ARGS() already fixes this, but do it
explicitly as well); we know it is safe to write one byte to outbuf,
because SECURE_ESCAPE_ARGS() guarantees it.

2/ If bytes was 1, the write to outbuf[1] was an off-by-one overflow.

3/ Do not call escape_str() with a 0 bufsize if bytes == overhead.

4/ Prevent various buffer overflows if bytes <= overhead.

0050-proc/escape.c: Prevent integer overflows in escape_str_utf8().

Simply rearrange the old comparisons. The new comparisons are safe,
because we know from previous checks that:

1/ wlen > 0

2/ my_cells < *maxcells (also: my_cells >= 0 and *maxcells > 0)

3/ len > 1

4/ my_bytes+1 < bufsize (also: my_bytes >= 0 and bufsize > 0)

0049-proc/escape.c: Handle negative wcwidth() return value.

This should never happen, because wcwidth() is called only if iswprint()
returns nonzero. But belt-and-suspenders, and make it visually clear
(very important for the next patch).

0048-proc/escape.c: Make sure all escape*() arguments are safe.

The SECURE_ESCAPE_ARGS() macro solves several potential problems
(although we found no problematic calls to the escape*() functions in
procps's code-base, but had to thoroughly review every call; and this is
library code):

1/ off-by-one overflows if the size of the destination buffer is 0;

2/ buffer overflows if this size (or "maxroom") is negative;

3/ integer overflows (for example, "*maxcells+1");

4/ always null-terminate the destination buffer (unless its size is 0).

---------------------------- adapted for newlib branch
. the escape.c now has just a single exported function
. thus SECURE_ESCAPE_ARGS() is needed in only 2 places
. unlike that original patch, macro is executed 1 time
( not like 'escape_command' calling 'escape_strlist' )
( which might then call 'escape_str' multiple times! )

Signed-off-by: Jim Warner <james.warner@comcast.net>