Commit Graph

1928 Commits

Author SHA1 Message Date
Qualys Security Advisory
97408d8b10 ps/output.c: Remove the page_shift variable.
It is static and not used anywhere.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
e66bf564f8 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.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
bb9c217f29 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.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
136e372495 ps/output.c: Fix outbuf overflows in pr_args() etc.
Because there is usually less than OUTBUF_SIZE available at endp.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
d31f5eb545 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).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
62f19dc5df proc/escape.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).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
7efa102248 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.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
37ce162604 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)
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
8d359b04ab 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).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
47303a3592 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).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
00ab5f0b32 proc/whattime.c: Always initialize buf.
In the human_readable case; otherwise the strcat() that follows may
append bytes to the previous contents of buf.

Also, slightly enlarge buf, as it was a bit too tight.

Could also replace all sprintf()s with snprintf()s, but all the calls
here output a limited number of characters, so they should be safe.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
7382ac88d5 proc/slab.c: Initialize struct slab_info in get_slabnode().
Especially its "next" member: this is what caused the crash in "slabtop:
Reset slab_list if get_slabinfo() fails." (if parse_slabinfo*() fails in
sscanf(), for example, then curr is set to NULL but it is already linked
into the "list" and its "next" member was never initialized).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
a33be33885 proc/sysinfo.c: Fix off-by-one in get_pid_digits().
At "pidbuf[rc] = '\0';" if "rc = read()" returns "sizeof pidbuf"
(unlikely to ever happen, but still).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
8136a7a664 proc/sysinfo.c: Prevent integer overflow of realloc() size. 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
5b6ab39c6d proc/slab.c: Check correct number of items after sscanf(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
3ccc6ed262 proc/slab.h: Fix off-by-one overflow in sscanf().
In proc/slab.c, functions parse_slabinfo20() and parse_slabinfo11(),
sscanf() might overflow curr->name, because "String input conversions
store a terminating null byte ('\0') to mark the end of the input; the
maximum field width does not include this terminator."

Add one byte to name[] for this terminator.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
bf12b14db9 proc/sig.c: Harden print_given_signals().
And signal_name_to_number().
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
3244e7ddb0 proc/devname.c: Never write more than "chop" (part 2).
"chop" is the maximum offset where the null-byte should be written;
respect this even if about to write just one (non-null) character.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
6b7ceb36a4 proc/devname.c: Never write more than "chop" characters.
This should be guaranteed by "tmp[chop] = '\0';" and "if(!c) break;" but
this patch adds a very easy belt-and-suspenders check.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
730bdc33e7 proc/devname.c: Prevent off-by-one overflow in dev_to_tty(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
9f59bd5c52 proc/devname.c: Use snprintf() in link_name().
Found no problematic use case at the moment, but better safe than sorry.
Also, return an error on snprintf() or readlink() truncation.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
59666e6255 proc/version.h: Protect parameter in LINUX_VERSION() macro.
Just in case (no problematic use case at the moment).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
f1077b7a55 proc/alloc.*: Use size_t, not unsigned int.
Otherwise this can truncate sizes on 64-bit platforms, and is one of the
reasons the integer overflows in file2strvec() are exploitable at all.
Also: catch potential integer overflow in xstrdup() (should never
happen, but better safe than sorry), and use memcpy() instead of
strcpy() (faster).

Warnings:

- in glibc, realloc(ptr, 0) is equivalent to free(ptr), but not here,
  because of the ++size;

- here, xstrdup() can return NULL (if str is NULL), which goes against
  the idea of the xalloc wrappers.

We were tempted to call exit() or xerrx() in those cases, but decided
against it, because it might break things in unexpected places; TODO?
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
98b79d1ef1 proc/alloc.c: Use vfprintf(), not fprintf().
This can disclose information from the stack, but is unlikely to have a
security impact in the context of the procps utilities:

user@debian:~$ w 2>&1 | xxd
00000000: a03c 79b7 1420 6661 696c 6564 2074 6f20  .<y.. failed to
00000010: 616c 6c6f 6361 7465 2033 3232 3137 3439  allocate 3221749
00000020: 3738 3020 6279 7465 7320 6f66 206d 656d  780 bytes of mem
00000030: 6f72 79                                  ory
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
7941bb512a proc/readproc.c: Add checks to get_ns_name() and get_ns_id(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
3ce9f837a3 proc/sig.c: Fix the strtosig() function.
Do not memleak "copy" in case of an error.

Do not use "sizeof(converted)" in snprintf(), since "converted" is a
"char *" (luckily, 8 >= sizeof(char *)). Also, remove "sizeof(char)"
which is guaranteed to be 1 by the C standard, and replace 8 with 12,
which is enough to hold any stringified int and does not consume more
memory (in both cases, the glibc malloc()ates a minimum-sized chunk).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
7367c4b1fd skill: Do not scan past the null-terminator in check_proc(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
a9ee0bf622 skill: Check return value of str*chr() in check_proc(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
52673d2fc7 skill: Properly null-terminate buf in check_proc().
Right now, if read() returns less than 127 bytes (the most likely case),
the end of the "string" buf will contain garbage from the stack, because
buf is always null-terminated at a fixed offset 127. This is especially
bad because the next operation is a strrchr().

Also, make sure that the whole /proc/PID/stat file is read, otherwise
its parsing may be unsafe (the strrchr() may point into user-controlled
data, comm). This should never happen with the current file format (comm
is very short), but be safe, just in case.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
37547e9f5f skill: Check the return value of fstat(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
858df7cc89 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.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
56e696ca5f 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).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
b019fdba5c skill: Remove unused NEXTARG macro. 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
0e1964bfbc 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)").
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
453e1df5d1 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."
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
7d6977b6f7 skill: Simplify the kill_main() loop.
Right now the "loop=0; break;" is never reached.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
daec51a06c pwdx: Fix a misleading comment.
It sounds like an off-by-one, but the code itself is correct.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
6df9ffb341 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().
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
ab8b3881a0 pidof: Do not memleak pidof_root if multiple -c options. 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
bba9f384c0 pidof: Do not skip the NULL terminator in cmdline.
This should never happen (cmdline[0] should always be non-NULL), but
just in case.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
6cadda2b4f pidof: Get the arg1 base name with get_basename().
Same as program_base, cmd_arg0base, and exe_link_base.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
6f2f033142 pidof: Do not memleak the contents of proc_t.
Just like "pgrep: Do not memleak the contents of proc_t."
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
031bc56f65 tload: Prevent integer overflows of ncols, nrows, and scr_size.
Also, use xerrx() instead of xerr() since errno is not set.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
4c346cf594 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.
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
d5442e10a7 tload: Use snprintf() instead of sprintf(). 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
afbb0f4886 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."
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
7664d9f306 tload: Use standard names instead of numbers. 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
5f3c071cd3 slabtop: Reset slab_list if get_slabinfo() fails.
Otherwise "the state of 'list' and 'stats' are undefined" (as per
get_slabinfo()'s documentation) and free_slabinfo() crashes (a
use-after-free).
2018-05-19 07:32:21 +10:00
Qualys Security Advisory
2fc42db322 uptime: Check the return value of various functions. 2018-05-19 07:32:21 +10:00
Qualys Security Advisory
b51ca2a1f8 pgrep: Prevent a potential stack-based buffer overflow.
This is one of the worst issues that we found: if the strlen() of one of
the cmdline arguments is greater than INT_MAX (it is possible), then the
"int bytes" could wrap around completely, back to a very large positive
int, and the next strncat() would be called with a huge number of
destination bytes (a stack-based buffer overflow).

Fortunately, every distribution that we checked compiles its procps
utilities with FORTIFY, and the fortified strncat() detects and aborts
the buffer overflow before it occurs.

This patch also fixes a secondary issue: the old "--bytes;" meant that
cmdline[sizeof (cmdline) - 2] was never written to if the while loop was
never entered; in the example below, "ff" is the uninitialized byte:

((exec -ca `python3 -c 'print("A" * 131000)'` /usr/bin/cat < /dev/zero) | sleep 60) &
pgrep -a -P "$!" 2>/dev/null | hexdump -C
00000000  31 32 34 36 30 20 41 41  41 41 41 41 41 41 41 41  |12460 AAAAAAAAAA|
00000010  41 41 41 41 41 41 41 41  41 41 41 41 41 41 41 41  |AAAAAAAAAAAAAAAA|
*
00001000  41 41 41 41 ff 0a 31 32  34 36 32 20 73 6c 65 65  |AAAA..12462 slee|
00001010  70 20 36 30 0a                                    |p 60.|
2018-05-19 07:32:21 +10:00