Commit Graph

1057 Commits

Author SHA1 Message Date
Iker Pedrosa
8281c82e32 usermod.c: fix covscan RESOURCE_LEAK
Error: RESOURCE_LEAK (CWE-772): [#def31]
shadow-4.8.1/src/usermod.c:813: alloc_fn: Storage is returned from allocation function "__gr_dup".
shadow-4.8.1/src/usermod.c:813: var_assign: Assigning: "ngrp" = storage returned from "__gr_dup(grp)".
shadow-4.8.1/src/usermod.c:892: leaked_storage: Variable "ngrp" going out of scope leaks the storage it points to.
  890|   		}
  891|   	}
  892|-> }
  893|
  894|   #ifdef SHADOWGRP

Error: RESOURCE_LEAK (CWE-772): [#def32]
shadow-4.8.1/src/usermod.c:933: alloc_fn: Storage is returned from allocation function "__sgr_dup".
shadow-4.8.1/src/usermod.c:933: var_assign: Assigning: "nsgrp" = storage returned from "__sgr_dup(sgrp)".
shadow-4.8.1/src/usermod.c:1031: leaked_storage: Variable "nsgrp" going out of scope leaks the storage it points to.
 1029|   		}
 1030|   	}
 1031|-> }
 1032|   #endif				/* SHADOWGRP */
 1033|

Error: RESOURCE_LEAK (CWE-772): [#def34]
shadow-4.8.1/src/usermod.c:1161: alloc_fn: Storage is returned from allocation function "getgr_nam_gid".
shadow-4.8.1/src/usermod.c:1161: var_assign: Assigning: "grp" = storage returned from "getgr_nam_gid(optarg)".
shadow-4.8.1/src/usermod.c:1495: leaked_storage: Variable "grp" going out of scope leaks the storage it points to.
 1493|   	}
 1494|   #endif				/* ENABLE_SUBIDS */
 1495|-> }
 1496|
 1497|   /*

Error: RESOURCE_LEAK (CWE-772): [#def35]
shadow-4.8.1/src/usermod.c:1991: 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/usermod.c:1991: var_assign: Assigning: "fd" = handle returned from "open("/var/log/lastlog", 2)".
shadow-4.8.1/src/usermod.c:2000: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2000: noescape: Resource "fd" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/usermod.c:2003: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2032: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
 2030|   		}
 2031|   	}
 2032|-> }
 2033|
 2034|   /*

Error: RESOURCE_LEAK (CWE-772): [#def36]
shadow-4.8.1/src/usermod.c:2052: 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/usermod.c:2052: var_assign: Assigning: "fd" = handle returned from "open("/var/log/faillog", 2)".
shadow-4.8.1/src/usermod.c:2061: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2061: noescape: Resource "fd" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/usermod.c:2064: noescape: Resource "fd" is not freed or pointed-to in "lseek".
shadow-4.8.1/src/usermod.c:2092: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
 2090|   		}
 2091|   	}
 2092|-> }
 2093|
 2094|   #ifndef NO_MOVE_MAILBOX
2021-06-11 11:50:49 +02:00
Serge Hallyn
9d37173b24 usermod, newusers, prefix: enforce absolute paths for homedir
useradd already was enforcing this, but these were not.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-06-01 22:12:24 -05:00
Serge Hallyn
8eb6f8ace4
Merge pull request #327 from squat/bugfix_relative_prefix_path
fix: create relative home path correctly
2021-05-29 14:16:46 -05:00
Serge Hallyn
9d169ffc41 fix newusers when nss provides subids
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)
2021-05-23 08:16:16 -05:00
Serge Hallyn
ea7af4e154
Merge pull request #340 from hallyn/2021-05-16/subidrange
Don't return owner in list_owner_ranges API call.
2021-05-22 18:16:43 -05:00
Serge Hallyn
3d670ba7ed nss/libsubid: simplify the ranges variable for list_owner_ranges
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>
2021-05-22 17:59:57 -05:00
Serge Hallyn
663824ef4c Fix useradd with SUB_UID_COUNT=0
Closes #298

Fix useradd when SUB_UID_COUNT=0 in login.defs.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-05-22 11:42:02 -05:00
Serge Hallyn
322db32971 Don't return owner in list_owner_ranges API call.
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>
2021-05-16 21:49:53 -05:00
Serge Hallyn
2b22a6909d libsubid: don't print error messages on stderr by default
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>
2021-05-15 12:38:55 -05:00
Christian Göttsche
eb1d2de0e9 set_selinux_file_context(): prepare context for actual file type
Search the SELinux selabel database for the file type to be created.
Not specifying the file mode can cause an incorrect file context to be
returned.

Also prepare contexts in commonio_close() for the generic database
filename, not with the backup suffix appended, to ensure the desired
file context after the final rename.

Closes: #322

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-05-06 16:58:10 +02:00
Christian Göttsche
c0aa8a876e vipw[selinux]: do not use deprecated typedef and skip context translation
This retrieved context is just passed to libselinux functions and not
printed or otherwise made available to the outside, so a context
translation to human readable MCS/MLS labels is not needed.
(see man:setrans.conf(5))

The typedef security_context_t is deprecated, see
9eb9c93275

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-05-06 16:58:10 +02:00
Christian Göttsche
6e4b2fe25d struct commonio_db[selinux]: do not use deprecated type security_context_t
The typedef security_context_t is deprecated, see
9eb9c93275

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>
2021-05-06 16:58:10 +02:00
Lucas Servén Marín
2c542f6c65
fix: create relative home path correctly
Currently, supplying a relative path via the --prefix flag to the
useradd command triggers a bug in the creation of home directories. The
code seems to unintentionally prepend a leading "/" to all paths,
quietly transforming a relative prefixed home path into an absolute
path. This can be seen in the following strace logs from running
"useradd --create-home --prefix tmp/root squat":

```
access("tmp/root//home/squat", F_OK)    = -1 ENOENT (No such file or directory)
access("/mp", F_OK)                     = 0
access("/mp/root", F_OK)                = 0
access("/mp/root/home", F_OK)           = 0
access("/mp/root/home/squat", F_OK)     = -1 ENOENT (No such file or directory)
mkdir("/mp/root/home/squat", 000)       = 0
chown("/mp/root/home/squat", 0, 0)      = 0
chmod("/mp/root/home/squat", 0755)      = 0
chown("tmp/root//home/squat", 1000, 1000) = -1 ENOENT (No such file or directory)
chmod("tmp/root//home/squat", 0700)     = -1 ENOENT (No such file or directory)
```

Note that the relative path is correctly probed in the beginning and it
is only during the recursive creation that the path is turned into an
absolute path. This invocation results in the creation of a "/mp"
hierarchy in the root of the filesystem.

Similar problems occur when using `--prefix ./tmp/root`.

This commit fixes the handling of relative paths by not assuming that
the given path is anchored with a "/".

Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
2021-04-29 15:06:53 +02:00
Serge Hallyn
0f4347d148 clean up libsubid headers
Move libsubid/api.h into libsubid/subid.h, and document the api in subid.h

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-04-16 21:03:08 -05:00
Serge Hallyn
8492dee663 subids: support nsswitch
Closes #154

When starting any operation to do with subuid delegation, check
nsswitch for a module to use.  If none is specified, then use
the traditional /etc/subuid and /etc/subgid files.

Currently only one module is supported, and there is no fallback
to the files on errors.  Several possibilities could be considered:

1. in case of connection error, fall back to files
2. in case of unknown user, also fall back to files

etc...

When non-files nss module is used, functions to edit the range
are not supported.  It may make sense to support it, but it also
may make sense to require another tool to be used.

libsubordinateio also uses the nss_ helpers.  This is how for instance
lxc could easily be converted to supporting nsswitch.

Add a set of test cases, including a dummy libsubid_zzz module.  This
hardcodes values such that:

'ubuntu' gets 200000 - 300000
'user1' gets 100000 - 165536
'error' emulates an nss module error
'unknown' emulates a user unknown to the nss module
'conn' emulates a connection error ot the nss module

Changes to libsubid:

Change the list_owner_ranges api: return a count instead of making the array
null terminated.

This is a breaking change, so bump the libsubid abi major number.

Rename free_subuid_range and free_subgid_range to ungrant_subuid_range,
because otherwise it's confusing with free_subid_ranges which frees
    memory.

Run libsubid tests in jenkins

Switch argument order in find_subid_owners

Move the db locking into subordinateio.c

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-04-16 21:02:37 -05:00
Serge Hallyn
514c1328b6 try again to fix libmisc sharing problem
Issue #297 reported seeing

*** Warning: Linking the shared library libsubid.la against the
*** static library ../libmisc/libmisc.a is not portable!

which commit b5fb1b38ee was supposed
to fix.  But a few commits later it's back.  So try to fix it
in the way the bug reporter suggested.  This broke builds some
other ways, namely a few missing library specifications, so add
those.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-04-11 17:42:04 -05:00
Haelwenn Monnier
b865e14f25
login & su: Treat an empty passwd field as invalid (#315)
* login & su: Treat an empty passwd field as invalid

Otherwise it's treated like the “require no password” clause while it probably
should be treated like a normal su that can't validate anyway.

A similar change should be done for USE_PAM.

* su & login: Introduce PREVENT_NO_AUTH
2021-03-28 22:16:03 -05:00
Serge Hallyn
697901a328
Merge pull request #303 from breard-r/yescrypt
Add yescrypt support
2021-03-28 22:13:56 -05:00
Serge Hallyn
7273c25cc2
Merge pull request #308 from martijndegouw/relaxgidcheck
newuidmap,newgidmap: Relax gid checking to allow running under alternative group ID
2021-03-02 12:42:25 -06:00
ikerexxe
0409c91a7f userdel: clarify "-f" usage
src/userdel.c: clarify the examples for "-f" option
2021-02-23 12:21:42 +01:00
Martijn de Gouw
c464ec5570 newuidmap,newgidmap: Relax gid checking to allow running under alternative group ID
Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
2021-02-08 13:32:18 +01:00
Geert Ijewski
fe159b7668 usermod: check if shell exists & is executable 2021-02-07 19:26:55 +01:00
Rodolphe Bréard
5cd04d03f9 Add yescrypt support 2021-02-01 22:11:10 +01:00
Serge Hallyn
0dffc7c612 useradd: don't try to create 0 subuids
Closes #289

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-01-01 13:10:12 -06:00
Serge Hallyn
c917ed7b76
Merge pull request #302 from whzhe51/br_master
useradd.c:fix memleaks of grp
2020-12-27 00:37:46 -06:00
Serge Hallyn
08f5577018
Merge pull request #301 from whzhe51/br_whzhe
useradd.c:fix memleak in get_groups
2020-12-27 00:31:30 -06:00
Serge Hallyn
1021195bfe
Merge pull request #299 from lnussel/master
useradd: use built-in settings by default
2020-12-27 00:26:47 -06:00
whzhe
fd9d79a1a3 useradd.c:fix memleak in get_groups
Signed-off-by: whzhe <wanghongzhe@huawei.com>
2020-12-20 22:05:03 -05:00
whzhe51
c44b71cec2 useradd.c:fix memleaks of grp
Signed-off-by: whzhe51 <wanghongzhe@huawei.com>
2020-12-20 20:14:49 -05:00
Ludwig Nussel
bbf4b79bc4 useradd: use built-in settings by default
Avoids installing inconsistent settings. The correct ones would be
written as soon as an admin uses useradd -D to modify the defaults.
2020-12-04 09:20:18 +01:00
ikerexxe
abb5c99114 useradd: log exit code when failing
src/useradd.c: log exit code when failing
2020-11-17 16:58:40 +01:00
Serge Hallyn
e7938d5a30
Merge pull request #291 from ikerexxe/covscan_issues
Two covscan issues
2020-11-08 16:33:55 -06:00
ikerexxe
569bd1d54f useradd: free grp to avoid leak
covscan issue:
Error: RESOURCE_LEAK (CWE-772): [#def39] [important]
src/useradd.c:728: alloc_fn: Storage is returned from allocation function "get_local_group".
src/useradd.c:728: var_assign: Assigning: "grp" = storage returned from "get_local_group(list)".
src/useradd.c:728: overwrite_var: Overwriting "grp" in "grp = get_local_group(list)" leaks the storage that "grp" points to.
726|   		 * GID values, otherwise the string is looked up as is.
727|   		 */
728|-> 		grp = get_local_group (list);
729|
730|   		/*
2020-10-27 11:42:34 +01:00
ikerexxe
2df8c0728d newgrp: delete dead code
covscan issue:
Error: CLANG_WARNING: [#def31]
src/newgrp.c:448:2: warning: Value stored to 'gid' is never read [deadcode.DeadStores]
	gid = getgid ();
2020-10-27 11:42:08 +01:00
Jamin W. Collins
bd4dc81a82 add parsing support for advertised force long opt
Existing help output advertises --force as a long opt.

  -f, --force                   delete group even if it is the primary group of a user

But errors when the long opt is used.

groupdel: unrecognized option '--force'

Signed-off-by: Jamin W. Collins <jamin.collins@gmail.com>
2020-10-17 09:18:30 -07:00
w00475903
e24700fd5d xfree: move xfree() function to xmalloc.c
Signed-off-by: whzhe <wanghongzhe@huawei.com>
2020-10-15 21:52:06 -04:00
Michael Mullin
dfd19fc35b Use {} to kep more in line with code style 2020-10-03 14:23:06 -04:00
Michael Mullin
0407fa8813 Fix the undefined xfree issue 2020-10-03 14:19:46 -04:00
Serge Hallyn
46ad1856ac
Merge pull request #277 from whzhe51/br_whzhe
gpasswd.c: fix memory leak in sg_adm
2020-10-02 19:37:20 -05:00
w00475903
22bfaf9e26 gpasswd.c: fix memory leak in sg_adm
Signed-off-by: whzhe <wanghongzhe@huawei.com>
2020-09-24 23:29:53 -04:00
ed neville
01a8df79b3 loop until waitpid returns pid_child or error
closes #104

Signed-off-by: ed neville <ed@s5h.net>
2020-09-20 23:04:11 +01:00
Serge Hallyn
9a10373ddb Revert "su.c: replace getopt with ad-hoc flag processing"
This reverts commit dc732e7734.
2020-08-28 15:16:11 -05:00
Serge Hallyn
b065fa4741 Revert "su.c: s/doshell/do_interactive_shell/"
This reverts commit 6f38f43fdd.
2020-08-28 15:16:04 -05:00
Serge Hallyn
3f35983656 Revert "su.c: implement --exec"
This reverts commit 4047d1fe8e.
2020-08-28 15:15:56 -05:00
Christian Brauner
ec98f190c1
Merge pull request #275 from hallyn/2020-08-27/test-su
Add tests on top of #254
2020-08-28 12:13:49 +02:00
Vito Caputo
4047d1fe8e su.c: implement --exec
It's now possible to run commands as other users without shell
interpolation by using "--exec":

Read /etc/shadow as root without specifying user:
```
su --exec /bin/cat -- /etc/shadow
```

Or specify user:
```
su --exec /bin/cat root -- /etc/shadow
```
2020-08-27 23:43:32 -05:00
Vito Caputo
6f38f43fdd su.c: s/doshell/do_interactive_shell/
Mechanical rename distinguishing this variable from intended changes
supporting executing commands without using an interpretive shell
(i.e. no '/bin/sh -c').
2020-08-27 23:43:29 -05:00
Vito Caputo
dc732e7734 su.c: replace getopt with ad-hoc flag processing
In preparation for supporting --exec I was testing the robustness
of "--" handling and it became apparent that things are currently
a bit broken in `su`.

Since "--" is currently of limited utility, as the subsequent
words are simply passed to the shell after "-c","command_string",
it seems to have gone unnoticed for ages.

However, with --exec, it's expected that "--" would be an almost
required separator with every such usage, considering the
following flags must be passed verbatim to execve() and will
likely begin with hyphens looking indistinguishable from any
other flags in lieu of shell interpolation to worry about.

For some practical context of the existing situation, this
invocation doesn't work today:
```
  $ su --command ls -- flags for shell
  No passwd entry for user 'flags'
  $
```

This should just run ls as root with "flags","for","shell"
forwarded to the shell after "-c","ls".

The "--" should block "flags" from being treated as the user.
That particular issue isn't a getopt one per-se, it's arguably
just a bug in su.c's implementation.

It *seemed* like an easy fix for this would be to add a check if
argv[optind-1] were "--" before treating argv[optind] as USER.

But testing that fix revealed getopt was rearranging things when
encountering "--", the "--" would always separate the handled
opts from the unhandled ones.  USER would become shifted to
*after* "--" even when it occurred before it!

If we change the command to specify the user, it works as-is:
```
  $ su --command ls root -- flags for shell
  Password:
  testfile
  $

```

But what's rather surprising is how that works; the argv winds up:

"su","--command","ls","--","root","flags","for","shell"

with optind pointing at "root".

That arrangement of argv is indistinguishable from omitting the
user and having "root","flags","for","shell" as the stuff after
"--".

This makes it non-trivial to fix the bug of omitting user
treating the first word after "--" as the user, which one could
argue is a potentially serious security bug if you omit the user,
expect the command to run as root, and the first word after "--"
is a valid user, and what follows that something valid and
potentially destructive not only running in unintended form but
as whatever user happened to be the first word after "--".

So, it seems like something important to fix, and getopt seems to
be getting in the way of fixing it properly without being more
trouble than replacing getopt.

In disbelief of what I was seeing getopt doing with argv here, I
took a glance at the getopt source and found the following:

```
      /* The special ARGV-element '--' means premature end of options.
	 Skip it like a null option,
	 then exchange with previous non-options as if it were an option,
	 then skip everything else like a non-option.  */

      if (d->optind != argc && !strcmp (argv[d->optind], "--"))
```

I basically never use getopt personally because ages ago it
annoyed me with its terrible API for what little it brought to
the table, and this brings it to a whole new level of awful.
2020-08-27 23:43:25 -05:00
Serge Hallyn
291c6fcc87
Merge pull request #267 from stoeckmann/chage
chage: Prevent signed integer overflows.
2020-08-13 00:34:19 -05:00
ed neville
3c9836a298 Removing trailing n typo
Signed-off-by: ed neville <ed@s5h.net>
2020-08-12 17:53:28 +01:00