Commit Graph

505 Commits

Author SHA1 Message Date
Alejandro Colomar
cdaa04e460 Remove uses of ulimit(3)
The function is obsolete.  It is recommended to use getrlimit(2) instead
(see the manual page for ulimit(3) or the POSIX manual for it).  Since
getrlimit(2) is required by POSIX.1-2001, we can rely on it.

Cc: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-15 16:22:05 -06:00
Alejandro Colomar
0527fa677b Add indentation to heavy use of preprocessor conditionals
This clarifies which code is under which conditions,
for further clenaup.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-15 16:22:05 -06:00
Guillem Jover
2a5b8810bb agetpass: Hook into build-system
Signed-off-by: Guillem Jover <guillem@hadrons.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-05 10:47:19 +01:00
Alex Colomar
155c9421b9 libmisc: agetpass(), erase_pass(): Add functions for getting passwords safely
There are several issues with getpass(3).

Many implementations of it share the same issues that the infamous
gets(3).  In glibc it's not so terrible, since it's a wrapper
around getline(3).  But it still has an important bug:

If the password is long enough, getline(3) will realloc(3) memory,
and prefixes of the password will be laying around in some
deallocated memory.

See the getpass(3) manual page for more details, and especially
the commit that marked it as deprecated, which links to a long
discussion in the linux-man@ mailing list.

So, readpassphrase(3bsd) is preferrable, which is provided by
libbsd on GNU systems.  However, using readpassphrase(3) directly
is a bit verbose, so we can write our own wrapper with a simpler
interface similar to that of getpass(3).

One of the benefits of writing our own interface around
readpassphrase(3) is that we can hide there any checks that should
be done always and which would be error-prone to repeat every
time.  For example, check that there was no truncation in the
password.

Also, use malloc(3) to get the buffer, instead of using a global
buffer.  We're not using a multithreaded program (and it wouldn't
make sense to do so), but it's nice to know that the visibility of
our passwords is as limited as possible.

erase_pass() is a clean-up function that handles all clean-up
correctly, including zeroing the entire buffer, and then
free(3)ing the memory.  By using [[gnu::malloc(erase_pass)]], we
make sure that we don't leak the buffers in any case, since the
compiler will be able to enforce clean up.

Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit?id=7ca189099d73bde954eed2d7fc21732bcc8ddc6b>
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-05 10:47:19 +01:00
Iker Pedrosa
d324c6776b libmisc: minimum id check for system accounts
The minimum id allocation for system accounts shouldn't be 0 as this is
reserved for root.

Signed-off-by: Tomáš Mráz <tm@t8m.info>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2022-10-06 20:09:35 -05:00
Alejandro Colomar
3dc1754e50 Use libc MAX() and MIN()
glibc, musl, FreeBSD, and OpenBSD define the MAX() and MIN()
macros in <sys/param.h> with the same definition that we use.
Let's not redefine it here and use the system one, as it's
effectively the same as we define (modulo whitespace).

See:

shadow (previously):

alx@asus5775:~/src/shadow/shadow$ grepc -ktm MAX
./lib/defines.h:318:#define MAX(x,y) (((x) > (y)) ? (x) : (y))

glibc:

alx@asus5775:~/src/gnu/glibc$ grepc -ktm -x 'sys/param.h$' MAX
./misc/sys/param.h:103:#define MAX(a,b) (((a)>(b))?(a):(b))

musl:

alx@asus5775:~/src/musl/musl$ grepc -ktm -x 'sys/param.h$' MAX
./include/sys/param.h:19:#define MAX(a,b) (((a)>(b))?(a):(b))

OpenBSD:

alx@asus5775:~/src/bsd/openbsd/src$ grepc -ktm -x 'sys/param.h$' MAX
./sys/sys/param.h:193:#define	MAX(a,b) (((a)>(b))?(a):(b))

FreeBSD:

alx@asus5775:~/src/bsd/freebsd/freebsd-src$ grepc -ktm -x 'sys/param.h$' MAX
./sys/sys/param.h:333:#define	MAX(a,b) (((a)>(b))?(a):(b))

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-09-30 16:13:36 -05:00
Alex Colomar
0d9799de04 Don't test for NULL before calling free(3)
free(3) accepts NULL, since the oldest ISO C.  I guess the
paranoid code was taking care of prehistoric implementations of
free(3).  I've never known of an implementation that doesn't
conform to this, so let's simplify this.

Remove xfree(3), which was effectively an equivalent of free(3).

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-09-29 16:03:53 +02:00
Samanta Navarro
cde221b858 copy_tree: carefully treat permissions
The setuid, setgid, and sticky bits are not copied during copy_tree.

Also start with very restrictive permissions before setting ownerships.

This prevents situations in which users in a group with less permissions
than others could win a race in opening the file before permissions are
removed again.

Proof of concept:

$ echo $HOME
/home/uwu
$ install -o uwu -g fandom -m 604 /dev/null /home/uwu/owo
$ ls -l /home/uwu/owo
-rw----r-- 1 uwu fandom 0 Sep  4 00:00 /home/uwu/owo

If /tmp is on another filesystem, then "usermod -md /tmp/uwu uwu" leads
to this temporary situation:

$ ls -l /tmp/uwu/owo
-rw----r-- 1 root root  0 Sep  4 00:00 /tmp/uwu/owo

This means that between openat and chownat_if_needed a user of group
fandom could open /tmp/uwu/owo and read the content when it is finally
written into the file.
2022-09-14 10:11:32 +02:00
Samanta Navarro
10cd68e0f0 copy_tree: do not block on fifos
Fixes regression introduced in faeab50e71.

If a directory contains fifos, then openat blocks until the other side
of the fifo is connected as well.

This means that users can prevent "usermod -m" from completing if their
home directories contain at least one fifo.
2022-09-09 15:19:12 +02:00
Samanta Navarro
f3bdb28e57 copy_tree: use fchmodat instead of chmod
Fixes regression introduced in faeab50e71
for setups configured without acl support.
2022-09-09 15:19:12 +02:00
Alexander Kanavin
cfc981df2a shadow: use relaxed usernames
The groupadd from shadow does not allow upper case group names, the
same is true for the upstream shadow. But distributions like
Debian/Ubuntu/CentOS has their own way to cope with this problem,
this patch is picked up from Fedora [1] to relax the usernames
restrictions to allow the upper case group names, and the relaxation is
POSIX compliant because POSIX indicate that usernames are composed of
characters from the portable filename character set [A-Za-z0-9._-].

[1] https://src.fedoraproject.org/rpms/shadow-utils/blob/rawhide/f/shadow-4.8-goodname.patch

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
2022-09-02 20:27:14 -05:00
Christian Göttsche
faeab50e71 Avoid races in copy_tree()
Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.

Introduce a path_info struct with the full path and dirfd and name
information for *at() functions, since the full path is needed for link
resolution, SELinux label lookup and ACL attributes.
2022-08-17 12:34:01 -05:00
Christian Göttsche
6cbec2d0aa Address minor compiler warnings
copydir.c:666:44: warning: unsigned conversion from 'int' to '__mode_t' {aka 'unsigned int'} changes value from '-4096' to '4294963200' [-Wsign-conversion]
      666 |         if (   (mknod (dst, statp->st_mode & ~07777, statp->st_rdev) != 0)
          |                                            ^

    copydir.c:116:1: warning: missing initializer for field 'quote' of 'struct error_context' [-Wmissing-field-initializers]
      116 | };
          | ^
    In file included from copydir.c:27:
    /usr/include/attr/error_context.h:30:23: note: 'quote' declared here
       30 |         const char *(*quote) (struct error_context *, const char *);
          |                       ^~~~~
2022-08-17 12:34:01 -05:00
Christian Göttsche
f606314f0c More robust file content copy in copy_tree()
Bail out on read(2) failure, continue on EINTR, support short writes and
increase chunk size.
2022-08-17 12:34:01 -05:00
Christian Göttsche
1d281273b1 Fail if regular file pre-exists in copy_tree()
Similar to the default behavior of mkdir(2), symlink(2), link(2) and
mknod(2).
2022-08-17 12:34:01 -05:00
Christian Göttsche
dab764d019 Require symlink support
Require lstat(2), lchown(2), S_IFLNK and S_ISLNK from POSIX.1-2001.

Already unconditionally used in lib/tcbfuncs.c and lib/run_part.c.
2022-08-17 12:34:01 -05:00
Christian Göttsche
f6f8bcd2a5 Avoid races in remove_tree()
Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.
2022-08-17 12:34:01 -05:00
Christian Göttsche
e9ae247cb1 Avoid races in chown_tree()
Use *at() functions to pin the directory operating in to avoid being
redirected by unprivileged users replacing parts of paths by symlinks to
privileged files.
2022-08-17 12:34:01 -05:00
Celeste Liu
6448da507e libmisc/root_flag: add tips for --root flag only support abspath
- Add tips in error message.
- Add tips in man.
- Add zh_CN and zh_TW for tips.

Signed-off-by: Celeste Liu <coelacanthus@outlook.com>
2022-08-06 15:04:06 -05:00
Christian Göttsche
c6c8130db4 Use function format attribute where applicable
Allow the compiler to verify the format string against the supplied
arguments.

    chage.c:239:51: warning: format not a string literal, format string not checked [-Wformat-nonliteral]
      239 |                 (void) strftime (buf, sizeof buf, format, tp);
          |                                                   ^~~~~~
2022-08-06 11:27:56 -05:00
Christian Göttsche
87d5a54ba0 Drop superfluous const from return type
salt.c:102:22: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
      102 | static /*@observer@*/const unsigned long SHA_get_salt_rounds (/*@null@*/int *prefered_rounds);
          |                      ^~~~~
    salt.c:110:22: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
      110 | static /*@observer@*/const unsigned long YESCRYPT_get_salt_cost (/*@null@*/int *prefered_cost);
          |                      ^~~~~

    subordinateio.c:160:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
      160 | static const bool range_exists(struct commonio_db *db, const char *owner)
          |        ^~~~~
2022-08-06 11:27:56 -05:00
Christian Göttsche
8f093ea93a Add include for uid_t
Allow IDEs to parse the header file on its own.
2022-08-06 11:27:56 -05:00
Christian Göttsche
4c641c1f2a Drop unnecessary prototype
The function is defined directly after.
2022-08-06 11:27:56 -05:00
Christian Göttsche
ae38d3a87f Declare read-only data const 2022-08-06 11:27:56 -05:00
Christian Göttsche
44917600b6 Drop register keyword
Compilers are free to ignore the indented hint and modern optimizations
should create good code by themself.

(As such it is for example deprecated in C++17.)
2022-08-06 11:27:56 -05:00
Christian Göttsche
c5090d91a1 Return void pointer from xmalloc
xmalloc is a wrapper around malloc(3), which bails out on OOM failures.
As such it returns raw memory and is used to allocated all kind of
types.
2022-08-06 11:27:56 -05:00
Christian Göttsche
e32b4a9a81 Declare read-only parameters const
Signal callers arguments are not going to be modified and allow passing
const pointers.
2022-08-06 11:27:56 -05:00
Xi Ruoyao
274e786be9 libmisc: use /dev/urandom as a generic fallback for read_random_bytes()
On systems with Linux kernel < 3.17, getentropy() and getrandom() may
exist but return ENOSYS.  Use /dev/urandom as a fallback to avoid a hard
requirement on Linux kernel version.

Fixes #512.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
2022-06-19 09:16:38 -05:00
Iker Pedrosa
0b51cde162 Remove commented out code and FIXMEs
In order to remove some of the FIXMEs it was necessary to change the
code and call getulong() instead of getlong().

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2022-05-24 07:49:11 -05:00
juyin
a43d0b95c4 libmisc: add check fopen return value in read_random_bytes()
Returns null when fopen fails. Then, using fread with a null pointer will cause a segfault.

Signed-off-by: Yan Zhu <zhuyan34@huawei.com>
2022-04-03 21:07:09 -05:00
Niko
e9bf727253 Handle ERANGE error correctly
The reentrant functions getgrgid_r, getgrnam_r, getpwnam_r, etc. all return an error code instead of setting errno. Adapt the error check accordingly.
2022-03-18 20:24:10 -05:00
Alejandro Colomar
fd5945e533 Use 'void' instead of 'RETSIGTYPE'. Use 'sighandler_t' too.
C89 and POSIX.1-2001 define signal(2) as returning a pointer to a
function returning 'void'.  K&R C signal(2) signature is obsolete.
Use 'void' directly.

Also, instead of writing the function pointer type explicitly, use
POSIX's 'sighandler_t'.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2022-01-15 08:25:53 -06:00
Alejandro Colomar
5450f9a904 Remove old compatibility DIRECT macro.
Use struct dirent directly.  See parent commit.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2022-01-15 08:25:53 -06:00
Serge Hallyn
39eea79d8d
Merge pull request #487 from cgzones/misc_warnings
Resolve several compiler warnings
2022-01-03 09:45:12 -06:00
Serge Hallyn
98f943f2a5
Merge pull request #481 from alejandro-colomar/STDC_HEADERS
Assume C89 is available
2022-01-03 09:37:06 -06:00
Christian Göttsche
119cee142e Declare argument of nss_init const
nss_init() does not modify its path argument, thus declare it const.
Also drop superfluous prototype.

nss.c:54:31: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   54 |                 nsswitch_path = NSSWITCH;
      |                               ^
2022-01-03 15:09:17 +01:00
Christian Göttsche
946eb84182 Do not drop const qualifier for Basename
The private Basename() implementation does not modify its argument, so
a cast to a non-const char pointer is not necessary.

newgrp.c:790:39: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
  790 |         progbase = (char *) Basename ((char *) prog);
      |                                       ^
newgrp.c:790:20: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
  790 |         progbase = (char *) Basename ((char *) prog);
      |                    ^

shell.c:48:70: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   48 |                 (void) snprintf (arg0, sizeof arg0, "-%s", Basename ((char *) file));
      |                                                                      ^
2022-01-03 15:09:17 +01:00
Christian Göttsche
45bba0e190 Use strict prototypes
Function declarations with no argument declare functions taking an
arbitrary number of arguments. Use the special type void to declare
functions taking no argument.
2022-01-03 15:09:17 +01:00
Serge Hallyn
e8a2cfa7dc
Merge pull request #451 from hallyn/2021-12-05/license 2022-01-02 18:38:42 -06:00
ed neville
b4472167c2 Adding nofollow to opens 2022-01-01 21:13:41 +00:00
Alejandro Colomar
8f134c0bea Use isdigit(3) instead of a reimplementation of it
C89 defined isdigit as a function that tests for any decimal-digit
character, defining the decimal digits as 0 1 2 3 4 5 6 7 8 9.

I don't own a copy of C89 to check, but check in C17:

7.4.1.5
5.2.1

More specifically:

> In both the source and execution basic character sets, the value
> of each character after 0 in the above list of decimal digits
> shall be one greater than the value of the previous.

And since in ascii(7), the character after '9' is ':', it's highly
unlikely that any implementation will ever accept any
_decimal digit_ other than 0..9.

POSIX simply defers to the ISO C standard.

This is exactly what we wanted from ISDIGIT(c), so just use it.
Non-standard implementations might have been slower or considered
other characters as digits in the past, but let's assume
implementations available today conform to ISO C89.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
44126d85ee Remove definition of ISDIGIT_LOCALE(c)
It wasn't being used at all.  Let's remove it.

Use isdigit(3) directly in comments that referenced it.

Also, in those comments, remove an outdated reference to the fact
that ISDIGIT_LOCALE(c) might evaluate its argument more than once,
which could be true a few commits ago, until
IN_CTYPE_DEFINITION(c) was removed.  Previously, the definition
for ISDIGIT_LOCALE(c) was:

 #if defined (STDC_HEADERS) || (!defined (isascii) && !defined (HAVE_ISASCII))
 # define IN_CTYPE_DOMAIN(c) 1
 #else
 # define IN_CTYPE_DOMAIN(c) isascii(c)
 #endif

 #define ISDIGIT_LOCALE(c) (IN_CTYPE_DOMAIN (c) && isdigit (c))

Which could evaluate 'c' twice on pre-C89 systems (which I hope
don't exist nowadays).

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
2a41a72b8c Use standard isspace(3), isalpha(3), and isupper(3)
Due to the recent removal of IN_CTYPE_DOMAIN(), the uppercase
macros that wrapped these standard calls are now defined to be
equivalent.  Therefore, there's no need for the wrappers, and it
is much more readable to use the standard calls directly.

However, hold on with ISDIGIT*(), since it's not so obvious what
to do with it.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
45d2e6dff0 Remove IN_CTYPE_DOMAIN, which was always true
The recent removal of STDC_HEADERS made IN_CTYPE_DOMAIN be defined
to 1 unconditionally.  Remove the now unnecessary definition, and
propagate its truthness to expressions where it was used.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
4e1afcd662 Assume STDC_HEADERS will always be defined
We're in 2021.  C89 is everywhere; in fact, there are many other
assumptions in the code that wouldn't probably hold on
pre-standard C environments.  Let's simplify and assume that C89
is available.

The specific assumptions are that:
- <string.h>, and <stdlib.h> are available
- strchr(3), strrchr(3), and strtok(3) are available
- isalpha(3), isspace(3), isdigit(3), and isupper(3) are available

I think we can safely assume we have all of those.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:39:04 +01:00
Serge Hallyn
9e5a852ee0
Merge pull request #480 from alejandro-colomar/memcpy
Remove HAVE_MEMCPY and HAVE_MEMSET ifdefs
2021-12-27 19:10:48 -06:00
Alejandro Colomar
047bfc47c6 Remove HAVE_MEMCPY ifdefs
memcpy(3) has been in standard C since C89.  It is also in
POSIX.1-2001, in SVr4, and in 4.3BSD (see memcpy(3) and memcpy(3p)).
We can assume that this function is always available.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-27 21:38:47 +01:00
Alejandro Colomar
3e602b58a2 Remove HAVE_STRFTIME ifdefs
strftime(3) has been in standard C since C89.  It is also in
POSIX.1-2001, and in SVr4 (see strftime(3) and strftime(3p)).
We can assume that this function is always available.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-27 20:50:13 +01:00
Serge Hallyn
e1b1d187f4
Merge pull request #467 from alejandro-colomar/date_to_str
Have a single definition of date_to_str()
2021-12-27 09:53:00 -06:00
Alejandro Colomar
355ad6a9e0 Have a single definition of date_to_str()
PARAMETERS:

According to the C2x charter, I reordered the parameters 'size'
and 'buf' from previously existing date_to_str() definitions.

C2x charter:
> 15. Application Programming Interfaces (APIs) should be
> self-documenting when possible.  In particular, the order of
> parameters in function declarations should be arranged such that
> the size of an array appears before the array.  The purpose is to
> allow Variable-Length Array (VLA) notation to be used.  This not
> only makes the code's purpose clearer to human readers, but also
> makes static analysis easier.  Any new APIs added to the Standard
> should take this into consideration.

I used 'long' for the date parameter, as some uses of the function
need to pass a negative value meaning "never".

FUNCTION BODY:

I didn't check '#ifdef HAVE_STRFTIME', which old definitions did,
since strftime(3) is guaranteed by the C89 standard, and all of
the conversion specifiers that we use are also specified by that
standard, so we don't need any extensions at all.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-26 18:55:39 +01:00