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.)
IDs already populate /etc/subuid and /etc/subgid files so it's necessary
not only to check for the owner name but also for the owner ID of a
given range.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2093311
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
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>
If /etc/nsswitch.conf doesn't exist podman crashes because shadow_logfd
is NULL. In order to avoid that load the log file descriptor with the
log_get_logfd() helper function.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2038811
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Systems on which <sys/time.h> conflicted with <time.h> are obsolete.
This macro has been marked as obsolete by autoconf documentation.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
As autoconf documentation says, this macro is obsolescent, as no
current systems have the bug in S_ISDIR, S_ISREG, etc..
The affected systems were Tektronix UTekV, Amdahl UTS, and
Motorola System V/88.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
GNU autoconf documentation marks this macro as obsolescent, as
current systems are compatible with POSIX.
Simplify code to unconditionally include <sys/wait.h>, and don't
redefine WIFEXITSTATUS() and WIFEXITED(), since they are mandated
by POSIX.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
POSIX.1-2001 defines 'struct dirent' in <dirent.h>. It replaces
the old 'struct direct' found in BSDs. All of the systems that I
checked (including FreeBSD, NetBSD, and OpenBSD), now provide
<dirent.h> with 'struct dirent', as mandated by POSIX.
Since autoconf first checks <dirent.h> and only if it's missing it
checks other header files, it's clear that it will always find
<dirent.h>, so let's simplify.
GNU autoconf documentation declares this macro as obsolescent, and
acknowledges that all current systems with directory libraries
have <dirent.h>:
<https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Particular-Headers.html>
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
Compilers are allowed to and do optimize memset(3) calls away for
pointers not accessed in the future. Since the memzero wrappers purpose
is exactly to unconditionally override memory (e.g. for stored
passwords) do not implement via regular memset(3), but via either
memset_s(3), explicit_bzero(3) or a hand written implementation using
volatile pointers.
See https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations
run_part() and run_parts() do not modify their directory, name and
action arguments.
Also include the header in the implementation to provide the prototypes.
useradd.c:2495:59: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
2495 | if (run_parts ("/etc/shadow-maint/useradd-pre.d", (char*)user_name,
| ^
useradd.c:2495:24: warning: passing argument 1 of ‘run_parts’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
2495 | if (run_parts ("/etc/shadow-maint/useradd-pre.d", (char*)user_name,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from useradd.c:45:
../lib/run_part.h:2:22: note: expected ‘char *’ but argument is of type ‘const char *’
2 | int run_parts (char *directory, char *name, char *action);
| ~~~~~~^~~~~~~~~
useradd.c:2496:25: warning: passing argument 3 of ‘run_parts’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
2496 | "useradd")) {
| ^~~~~~~~~
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;
| ^
Function declarations with no argument declare functions taking an
arbitrary number of arguments. Use the special type void to declare
functions taking no argument.
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>
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>
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>
memset(3) has been in standard C since C89. It is also in
POSIX.1-2001, in SVr4, and in 4.3BSD (see memset(3) and memset(3p)).
We can assume that this function is always available.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
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>
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>
The build was failing with duplicate symbol errors with -fno-common.
This is the default in GCC 10 and later, and explicitly enabled in some
distributions to catch problems like this. There were two causes:
- Prog and shadow_logfd were defined in a header file that was included
in multiple other files. Fix this by defining them once in
shadowlog.c, and having extern declarations in the header.
- Most of the tools (except id/nologin) also define a Prog variable,
which is not intended to alias the one in the library. Fix
this by renaming Prog in the library to shadow_progname, which also
matches the new accessor functions for it.
Add an additional NULL check condition in spw_free() and pw_free() to
avoid freeing an already empty pointer.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
During shadowtcb_move() the directory is temporarily changed to be
owned by root:root with permissions 0700. After the change is done,
the ownership and permissions were supposed to be restored. The
call for chown() was there, but the chmod() call was missing. This
resulted in the broken TCB functionality. The added chmod() fixes
the issue.
Close the selabel handle to update the file_context. This means that the
file_context will be remmaped and used by selabel_lookup() to return
the appropriate context to label the home folder.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1993081
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
There's a better way to do this, and I hope to clean that up,
but this fixes out of tree builds for me right now.
Closes#386
Signed-off-by: Serge Hallyn <serge@hallyn.com>
The functions crypt(3), crypt_gensalt(3), and their
feature test macros may be defined in there.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
Error: RESOURCE_LEAK (CWE-772): [#def1]
shadow-4.8.1/lib/commonio.c:320: alloc_fn: Storage is returned from allocation function "fopen_set_perms".
shadow-4.8.1/lib/commonio.c:320: var_assign: Assigning: "bkfp" = storage returned from "fopen_set_perms(backup, "w", &sb)".
shadow-4.8.1/lib/commonio.c:329: noescape: Resource "bkfp" is not freed or pointed-to in "putc".
shadow-4.8.1/lib/commonio.c:334: noescape: Resource "bkfp" is not freed or pointed-to in "fflush".
shadow-4.8.1/lib/commonio.c:339: noescape: Resource "bkfp" is not freed or pointed-to in "fileno".
shadow-4.8.1/lib/commonio.c:342: leaked_storage: Variable "bkfp" going out of scope leaks the storage it points to.
340| || (fclose (bkfp) != 0)) {
341| /* FIXME: unlink the backup file? */
342|-> return -1;
343| }
344|
Error: RESOURCE_LEAK (CWE-772): [#def2]
shadow-4.8.1/libmisc/addgrps.c:69: alloc_fn: Storage is returned from allocation function "malloc".
shadow-4.8.1/libmisc/addgrps.c:69: var_assign: Assigning: "grouplist" = storage returned from "malloc(i * 4UL)".
shadow-4.8.1/libmisc/addgrps.c:73: noescape: Resource "grouplist" is not freed or pointed-to in "getgroups". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/libmisc/addgrps.c:126: leaked_storage: Variable "grouplist" going out of scope leaks the storage it points to.
124| }
125|
126|-> return 0;
127| }
128| #else /* HAVE_SETGROUPS && !USE_PAM */
Error: RESOURCE_LEAK (CWE-772): [#def3]
shadow-4.8.1/libmisc/chowntty.c:62: alloc_fn: Storage is returned from allocation function "getgr_nam_gid".
shadow-4.8.1/libmisc/chowntty.c:62: var_assign: Assigning: "grent" = storage returned from "getgr_nam_gid(getdef_str("TTYGROUP"))".
shadow-4.8.1/libmisc/chowntty.c:98: leaked_storage: Variable "grent" going out of scope leaks the storage it points to.
96| */
97| #endif
98|-> }
99|
Error: RESOURCE_LEAK (CWE-772): [#def4]
shadow-4.8.1/libmisc/copydir.c:742: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/libmisc/copydir.c:742: var_assign: Assigning: "ifd" = handle returned from "open(src, 0)".
shadow-4.8.1/libmisc/copydir.c:748: leaked_handle: Handle variable "ifd" going out of scope leaks the handle.
746| #ifdef WITH_SELINUX
747| if (set_selinux_file_context (dst, NULL) != 0) {
748|-> return -1;
749| }
750| #endif /* WITH_SELINUX */
Error: RESOURCE_LEAK (CWE-772): [#def5]
shadow-4.8.1/libmisc/copydir.c:751: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/libmisc/copydir.c:751: var_assign: Assigning: "ofd" = handle returned from "open(dst, 577, statp->st_mode & 0xfffU)".
shadow-4.8.1/libmisc/copydir.c:752: noescape: Resource "ofd" is not freed or pointed-to in "fchown_if_needed".
shadow-4.8.1/libmisc/copydir.c:775: leaked_handle: Handle variable "ofd" going out of scope leaks the handle.
773| ) {
774| (void) close (ifd);
775|-> return -1;
776| }
777|
Error: RESOURCE_LEAK (CWE-772): [#def7]
shadow-4.8.1/libmisc/idmapping.c:188: alloc_fn: Storage is returned from allocation function "xmalloc".
shadow-4.8.1/libmisc/idmapping.c:188: var_assign: Assigning: "buf" = storage returned from "xmalloc(bufsize)".
shadow-4.8.1/libmisc/idmapping.c:188: var_assign: Assigning: "pos" = "buf".
shadow-4.8.1/libmisc/idmapping.c:213: noescape: Resource "buf" is not freed or pointed-to in "write".
shadow-4.8.1/libmisc/idmapping.c:219: leaked_storage: Variable "pos" going out of scope leaks the storage it points to.
shadow-4.8.1/libmisc/idmapping.c:219: leaked_storage: Variable "buf" going out of scope leaks the storage it points to.
217| }
218| close(fd);
219|-> }
Error: RESOURCE_LEAK (CWE-772): [#def8]
shadow-4.8.1/libmisc/list.c:211: alloc_fn: Storage is returned from allocation function "xstrdup".
shadow-4.8.1/libmisc/list.c:211: var_assign: Assigning: "members" = storage returned from "xstrdup(comma)".
shadow-4.8.1/libmisc/list.c:217: var_assign: Assigning: "cp" = "members".
shadow-4.8.1/libmisc/list.c:218: noescape: Resource "cp" is not freed or pointed-to in "strchr".
shadow-4.8.1/libmisc/list.c:244: leaked_storage: Variable "cp" going out of scope leaks the storage it points to.
shadow-4.8.1/libmisc/list.c:244: leaked_storage: Variable "members" going out of scope leaks the storage it points to.
242| if ('\0' == *members) {
243| *array = (char *) 0;
244|-> return array;
245| }
246|
Error: RESOURCE_LEAK (CWE-772): [#def11]
shadow-4.8.1/libmisc/myname.c:61: alloc_fn: Storage is returned from allocation function "xgetpwnam".
shadow-4.8.1/libmisc/myname.c:61: var_assign: Assigning: "pw" = storage returned from "xgetpwnam(cp)".
shadow-4.8.1/libmisc/myname.c:67: leaked_storage: Variable "pw" going out of scope leaks the storage it points to.
65| }
66|
67|-> return xgetpwuid (ruid);
68| }
69|
Error: RESOURCE_LEAK (CWE-772): [#def12]
shadow-4.8.1/libmisc/user_busy.c:260: alloc_fn: Storage is returned from allocation function "opendir".
shadow-4.8.1/libmisc/user_busy.c:260: var_assign: Assigning: "task_dir" = storage returned from "opendir(task_path)".
shadow-4.8.1/libmisc/user_busy.c:262: noescape: Resource "task_dir" is not freed or pointed-to in "readdir".
shadow-4.8.1/libmisc/user_busy.c:278: leaked_storage: Variable "task_dir" going out of scope leaks the storage it points to.
276| _("%s: user %s is currently used by process %d\n"),
277| Prog, name, pid);
278|-> return 1;
279| }
280| }
Error: RESOURCE_LEAK (CWE-772): [#def20]
shadow-4.8.1/src/newgrp.c:162: alloc_fn: Storage is returned from allocation function "xgetspnam".
shadow-4.8.1/src/newgrp.c:162: var_assign: Assigning: "spwd" = storage returned from "xgetspnam(pwd->pw_name)".
shadow-4.8.1/src/newgrp.c:234: leaked_storage: Variable "spwd" going out of scope leaks the storage it points to.
232| }
233|
234|-> return;
235|
236| failure:
Error: RESOURCE_LEAK (CWE-772): [#def21]
shadow-4.8.1/src/passwd.c:530: alloc_fn: Storage is returned from allocation function "xstrdup".
shadow-4.8.1/src/passwd.c:530: var_assign: Assigning: "cp" = storage returned from "xstrdup(crypt_passwd)".
shadow-4.8.1/src/passwd.c:551: noescape: Resource "cp" is not freed or pointed-to in "strlen".
shadow-4.8.1/src/passwd.c:554: noescape: Resource "cp" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/passwd.c:555: overwrite_var: Overwriting "cp" in "cp = newpw" leaks the storage that "cp" points to.
553| strcpy (newpw, "!");
554| strcat (newpw, cp);
555|-> cp = newpw;
556| }
557| return cp;
Closes#331
1. drop 'has_any_range' nss method as it is not useful
2. do not try to create a subid range in newusers when using nss for
subids, since that's not possible.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
(cherry picked from commit 88a434adbdcf4a8640793fd58bcd2ba77598349d)
Following alexey-tikhonov's suggestion.
Since we've dropped the 'owner' field in the data returned for
get_subid_ranges, we can just return a single allocated array of
simple structs. This means we can return a ** instead of ***, and
we can get rid of the subid_free_ranges() helper, since the caller
can just free() the returned data.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Closes: 339
struct subordinate_range is pretty closely tied to the existing
subid code and /etc/subuid format, so it includes an owner. Dropping
that or even renaming it is more painful than I'd first thought.
So introduce a 'struct subid_range' which is only the start and
count, leaving 'struct subordinate_range' as the owner, start and
count.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Closes#325
Add a new subid_init() function which can be used to specify the
stream on which error messages should be printed. (If you want to
get fancy you can redirect that to memory :) If subid_init() is
not called, use stderr. If NULL is passed, then /dev/null will
be used.
This patch also fixes up the 'Prog', which previously had to be
defined by any program linking against libsubid. Now, by default
in libsubid it will show (subid). Once subid_init() is called,
it will use the first variable passed to subid_init().
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Include the new HMAC_CRYPTO_ALGO key that is needed by pam_timestamp to
select the algorithm that is going to be used to calculate the message
authentication code.
pam_timestamp is currently using an embedded algorithm to calculate the
HMAC message, but the idea is to improve this behaviour by relying on
openssl's implementation. On top of that, the ability to change the
algorithm with a simple configuration change allows to simplify the
process of removing unsecure algorithms.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1947294
Once opened, keep the selabel database open for further lookups.
Register an exit handler to close the database.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
