useradd generates an empty subid range when adding a new user. This is
caused because there are two variables, one local and the other one
global, that have a very similar name and they are used indistinctly in
the code. The local variable loads the SUB_*ID_COUNT configuration from
the login.defs file, while the global variable, which holds a value of
0, is used to generate the subid range. Causing the empty subid range
problem.
I've merged the two variables in the local one and removed the global
variable. I prefer to do it this way to reduce the scope of it but I'm
open to doing it the other way round.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1990653
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
When the -S and -a options are used for passwd to list the status
of all passwords, there is a chance the pw_passwd field of struct
passwd will be NULL. This can be due to 'files compat' being set
for passwd in /etc/nsswitch.conf and the usage of some features
not available in the 'files' mode (e.g. a plus sign at the start
of a line).
Example:
germ161:~ # grep passwd /etc/nsswitch.conf
passwd: files compat
germ161:~ # rpm -qa shadow
shadow-4.2.1-34.20.x86_64
germ161:~ # grep passwd /etc/nsswitch.conf
passwd: files compat
germ161:~ # grep + /etc/passwd
+@nisgroup
germ161:~ # passwd -S -a > /dev/null
Segmentation fault (core dumped)
With this commit:
germ161:~ # passwd -S -a > /dev/null
passwd: malformed password data obtained for user +@nisgroup
The only way of removing a group from the supplementary list is to use
-G option, and list all groups that the user is a member of except for
the one that wants to be removed. The problem lies when there's a user
that contains both local and remote groups, and the group to be removed
is a local one. As we need to include the remote group with -G option
the command will fail.
This reverts commit 140510de9d. This way,
it would be possible to remove the remote groups from the supplementary
list.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1967641
Resolves: https://github.com/shadow-maint/shadow/issues/338
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Fixes a link failure:
../libsubid/.libs/libsubid.so: undefined reference to `FascistCheck'
Bug: https://bugs.gentoo.org/806124
Signed-off-by: Mike Gilbert <floppym@gentoo.org>
new*idmap has a dependency with libeconf since commit
c464ec5570. I'm just adding it to the
Makefile to be able to compile in distributions that include libeconf.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Since bbf4b79, we stopped shipping /etc/default/useradd, and therefore
install of shadow does not auto-create /etc/default. So when useradd
tries to save a new default, it needs to create the directory.
Closes#390.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
libsubid's Makefile.am was always setting enable-shared in its LDFLAGS.
Do that only if not building static.
Closes#387
Signed-off-by: Serge Hallyn <shallyn@cisco.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>
In 9eb191edc4 I included a free() that
frees the members variable, which in turn causes the comma_to_list()
function to return an array of empty elements. The array variable holds
a list of pointers that point to offsets of the members variable. When
the function succeeds freeing members variable causes the elements of
the array variable to point to an empty string.
This is causing several regressions in our internal testing environment.
So, I'm reverting the change.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Most Linux distributions, including Fedora and RHEL 8, are shipping
with libxcrypt >= 4.0.
Since that version of libxcrypt the provided family of crypt_gensalt()
functions are able to use automatic entropy drawn from secure system
ressources, like arc4random(), getentropy() or getrandom().
Anyways, the settings generated by crypt_gensalt() are always
guaranteed to works with the crypt() function.
Using crypt_gensalt() is also needed to make proper use of newer
hashing methods, like yescrypt, provided by libxcrypt.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
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>
In a previous commit we introduced /dev/urandom as a source to obtain
random bytes from. This may not be available on all systems, or when
operating inside of a chroot.
Almost all systems provide functions to obtain random bytes from
secure system ressources. Thus we should prefer to use these, and
fall back to /dev/urandom, if there is no such function present, as
a last resort.
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;
Using the random() function to obtain pseudo-random bytes
for generating salt strings is considered to be dangerous.
See CWE-327.
We really should use a more reliable source for obtaining
pseudo-random bytes like /dev/urandom.
Fixes#376.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
In the previous commit we refactored the functions converting the
rounds number into a string for use with the crypt() function, to
not require any static buffer anymore.
Add some clarifying comments about how the minimum required buffer
length is computed inside of these functions.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
* Move all pre-processor defines to the top of the file.
* Unify the gensalt() function to be useable for all supported
hash methods.
* Drop the gensalt_{b,yes}crypt() functions in favor of the
previous change.
* Refactor the functions converting the rounds number into
a string for use with the crypt() function, to not require
any static buffer anymore.
* Clarify the comment about how crypt_make_salt() chooses the used
hash method from the settings in the login.defs file.
* Use memset() to fill static buffers with zero before using them.
* Use a fixed amount of 16 random base64-chars for the
sha{256,512}crypt hash methods, which is effectively still less
than the recommendation from NIST (>= 128 bits), but the maximum
those methods can effectively use (approx. 90 bits).
* Rename ROUNDS_{MIN,MAX} to SHA_ROUNDS_{MIN,MAX}.
* Bugfixes in the logic of setting rounds in BCRYPT_salt_rounds().
* Likewise for YESCRYPT_salt_cost().
* Fix formatting and white-space errors.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
The corresponding functions for the other hash methods all take
a pointer to an integer value as the only paramater, so this
particular function should do so as well.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
Error: RESOURCE_LEAK (CWE-772): [#def28]
shadow-4.8.1/src/useradd.c:1905: 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/src/useradd.c:1905: var_assign: Assigning: "fd" = handle returned from "open("/var/log/faillog", 2)".
shadow-4.8.1/src/useradd.c:1906: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/useradd.c:1917: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
1915| /* continue */
1916| }
1917|-> }
1918|
1919| static void lastlog_reset (uid_t uid)
Error: RESOURCE_LEAK (CWE-772): [#def29]
shadow-4.8.1/src/useradd.c:1938: 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/src/useradd.c:1938: var_assign: Assigning: "fd" = handle returned from "open("/var/log/lastlog", 2)".
shadow-4.8.1/src/useradd.c:1939: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/useradd.c:1950: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
1948| /* continue */
1949| }
1950|-> }
1951|
1952| static void tallylog_reset (const char *user_name)
Error: RESOURCE_LEAK (CWE-772): [#def30]
shadow-4.8.1/src/useradd.c:2109: alloc_fn: Storage is returned from allocation function "strdup".
shadow-4.8.1/src/useradd.c:2109: var_assign: Assigning: "bhome" = storage returned from "strdup(prefix_user_home)".
shadow-4.8.1/src/useradd.c:2131: noescape: Resource "bhome" is not freed or pointed-to in "strtok".
shadow-4.8.1/src/useradd.c:2207: leaked_storage: Variable "bhome" going out of scope leaks the storage it points to.
2205| }
2206| #endif
2207|-> }
2208| }
2209|
Following the discussion https://github.com/shadow-maint/shadow/pull/345
I have changed the documentation to clarify the behaviour of subid
delegation when any subid source except files is configured.
