Commit Graph

470 Commits

Author SHA1 Message Date
Jim Warner
35e9269bbe top: 'other filters' saved with config file, pgm logic
Well, after the rearranging and refactoring, all those
active 'other filter' entries for each window will now
be preserved in the user's configuration file via 'W'.

For raising the issue below, thanks to Marco Ippolito.

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

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
12b0b5b2cf top: refactored some more peripheral 'inspect' support
These modifications are being made now in anticipation
of some coming 'other filter' config file changes. Our
entries must be written last to the rc file since that
is where the users have been told to 'echo' additions.

Therefore, that 'config_insp' function must be adapted
to anticipate a passed buffer that was already primed.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
5e8bf3c028 top: refactored some of that 'other filtering' support
If we are to support preserving 'other filter' entries
in the rcfile, then the current logic setting up those
osel entries for a WIN_t must be shareable for startup
and when interacting with a user. So, this commit just
repositions this current code in a shareable function.

[ along the way, we give the prior guy a proper name ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
016f10cb93 top: reposition some of that 'other filtering' support
When we get around to saving that 'Other Filter' stuff
in the rcfile, we'll need access to the Fieldstab plus
the justify_pad() function. So this commit repositions
two 'osel' functions in anticipation of adding 1 more.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
8398b7d9b0 top: refactor some of that configuration files support
The 'config_file()' function was getting a little long
in the tooth, so this commit simply renames/rearranges
some stuff anticipating 'other filters' in the rcfile.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
567a9b6240 top: let's honor <Esc> key on color mapping screen too
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
775223a817 top: make rcfile duplicate fields check more efficient
Jeeze, there was no need to employ *both* strchr() and
strrchr() when ensuring fields hadn't been duplicated.

So let's avoid one of those function calls completely.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:25:17 +10:00
Jim Warner
80f9815f5f 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>
2018-06-09 21:45:38 +10:00
Jim Warner
5705866fb6 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>
2018-06-09 21:45:38 +10:00
Jim Warner
19ec80bd41 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>
2018-06-09 21:45:38 +10:00
Jim Warner
c3323bdb68 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>
2018-06-09 21:45:38 +10:00
Jim Warner
4d9e4ac4f6 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>
2018-06-09 21:35:20 +10:00
Jim Warner
fa96f3e5dc 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>
2018-06-09 21:35:20 +10:00
Jim Warner
34feb6183a 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>
2018-06-09 21:35:20 +10:00
Jim Warner
59c8c1c86c 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>
2018-06-09 21:35:20 +10:00
Jim Warner
8ce5f080a2 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>
2018-06-09 21:35:20 +10:00
Jim Warner
2c6a480cc8 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 2cfdbbe897

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-09 21:35:20 +10:00
Jim Warner
a24b369132 top: eliminate a couple of warnings of -Wunused-result
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-09 21:35:20 +10:00
Jim Warner
c314f9f953 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 55a42ae040

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-09 21:35:20 +10:00
Jim Warner
bd7e1fb343 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>
2018-06-09 21:35:20 +10:00
Jim Warner
8b94d11585 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>
2018-06-09 21:35:20 +10:00
Jim Warner
085351a0ee 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>
2018-06-09 21:35:20 +10:00
Jim Warner
ffee26c370 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>
2018-06-09 21:35:20 +10:00
Jim Warner
e691cbaef4 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>
2018-06-09 21:35:20 +10:00
Jim Warner
4550e60144 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>
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
d099d0e5a1 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().
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
2ba9c569e5 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).
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
6024543e79 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()).
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
74e9d0afe1 0112-top: Prevent integer overflow in adj_geometry(). 2018-06-09 21:35:20 +10:00
Qualys Security Advisory
34b08eb8ac 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.
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
2fabc50998 0110-top: Prevent integer overflows in config_file() and other_selection(). 2018-06-09 21:35:20 +10:00
Qualys Security Advisory
e1f419737f 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()."
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
022cda9599 0107-top: Initialize struct sigaction in before(). 2018-06-09 21:35:20 +10:00
Qualys Security Advisory
2c461c8b05 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.
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
f02fff62fb 0104-top: Initialize cp in task_show().
Found no problematic case at the moment, but this is a cheap
just-in-case.
2018-06-09 21:35:20 +10:00
Qualys Security Advisory
8b29093481 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>
2018-06-09 21:35:19 +10:00
Qualys Security Advisory
9cb8bee6a1 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.
2018-06-09 21:35:19 +10:00
Qualys Security Advisory
766e31a2c3 0100-top: Check Rc.fixed_widest.
Otherwise it leads to crashes (for example, setting it to 2147483600 in
the configuration file segfaults top).
2018-06-09 21:35:19 +10:00
Qualys Security Advisory
0b0356de5c 0098-top: Check i when setting Curwin in config_file().
Otherwise it leads to out-of-bounds reads (and maybe writes).
2018-06-09 21:35:19 +10:00
Qualys Security Advisory
7c92bff183 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.
2018-06-09 21:35:19 +10:00
Qualys Security Advisory
54f02b7e11 0096-top: Fix double-fclose() in configs_read().
It happens only if RCFILE_NOERR is defined (it is not, by default).
2018-06-09 21:35:19 +10:00
Jim Warner
b2d7f620c0 top: show that truncation indicator ('+') consistently
With a little luck, this should be the final tweak for
our support of extra wide characters. Currently, those
characters don't always display the '+' indicator when
they've been truncated. Now, it should always be seen.

[ plus it's done a tad more efficiently via snprintf ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-03-03 17:53:10 +11:00
Jim Warner
2119750a51 top: change to exploit a newly added UID used at login
In addition to exploiting the login user ID provision,
the following miscellaneous changes are also included:

. unnecessary braces have been eliminated from an 'if'

. a comment with case EU_CPU: was corrected to 's_int'
and the associated block of code relocated accordingly

. case EU_CPN: wasn't shared with other enumerators so
reference to 'i' was changed to that actual enumerator

. case EU_SGN: wasn't shared with other enumerators so
reference to 'i' was changed to that actual enumerator

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
52f0ee2c41 top: update copyright dates in source and man document
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
0f54ff181a top: try to avoid premature truncation indicator ('+')
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
3aa348ad77 top: avoid potential truncation with 'Inspect' feature
As it turns out, that Ukrainian 'demo' text supporting
the '=' command was 152 bytes long, up from an English
version of 80 bytes. Unfortunately, the buffer used to
format all such strings was insufficient at 128 bytes.

Depending on the width of one's terminal, some strange
result could be experienced when a multi-byte sequence
was truncated. So, this just makes that buffer bigger.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
603129e60a top: allow translated field headers to determine width
After wrestling with extra wide characters, supporting
languages like zh_CN, sometimes default/minimum column
widths might force a truncation of translated headers.

So, this commit explores one way that such truncations
could be avoided. It is designed so as to have minimal
impact on existing code, ultimately affecting just one
function. But it's off by default via its own #define.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
7e1dfc12b6 top: an efficiency tweak to extra wide character logic
When I recently added extra wide character support for
locales like zh_CN, I didn't worry about some overhead
associated with the new calls to 'mbtowc' & 'wcwidth'.
That's because such overhead was usually incurred with
user interactions, not a normal iterative top display.

There was, however, one area where this overhead would
impact the normal iterative top mode - that's with the
Summary display. So I peeked at the glibc source code.

As it turns out, the costs of executing those 'mbtowc'
and 'wcwidth' functions were not at all insignificant.
So, this patch will avoid them in the vast majority of
instances, while still enabling extra wide characters.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
0192197198 top: standardize width of the %CPU & %MEM columns at 5
There is (should be) no justification for changing the
width of the percentage columns (%CPU, %MEM) depending
on the BOOST_PERCNT #define. So this patch will ensure
that both columns are fixed at their former maximum 5.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:33:59 +11:00
Jim Warner
6a5cd6d6fb top: respond to an additional library 'state' category
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-12 20:58:31 +11:00