Commit Graph

3422 Commits

Author SHA1 Message Date
Alejandro Colomar fce1d88479 lib/list.c: is_on_list(): Call strsep(3) instead of open-coding it
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar 46fd68c37e lib/list.c: is_on_list(): Move break condition to loop controlling expression
This change executes `i++` one more time before breaking, so we need to
update the `i+1` after the loop to just `i`.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar fb01e07e83 lib/list.c: is_on_list(): Move code out of loop
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar 08ae38e394 lib/list.c: is_on_list(): Remove unnecessary use of temporary variable
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar 34b113baba lib/sgetspent.c: sgetspent(): Explicitly use an empty string literal
cp can only be an empty string literal in that conditional.  Use a
string literal to be more explicit.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar 93151689c0 lib/sgetspent.c: sgetspent(): Use NULL instead of 0 to mean a null pointer constant
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar ae17e0291d lib/port.c: getportent(): Call strpbrk(3) instead of open-coding it
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar 03677d9acf lib/: Call strsep(3) instead of open-coding it
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar 5f8f19f267 lib/: Call strchrnul(3) instead of open-coding it
Performance tests made in 2007 are obsolete.  We should assume libc is
reasonably fast today (otherwise, report a bug to libc).

$ git blame -- lib/sgetgrent.c | grep strchr
45c6603cc (nekral-guest      2007-10-07 11:44:02 +0000  30)  *	WARNING: I profiled this once with and without strchr() calls
6f88bcf58 (nekral-guest      2008-05-26 08:31:14 +0000  97) 		cp = strchr (cp, ':');

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 17:11:36 -05:00
Alejandro Colomar bed18501b1 lib/, src/: Call gmtime_r(3) instead of gmtime(3)
It's trivial to do the change, and it removes a CodeQL warning.
We don't need to be reentrant, but it doesn't hurt either.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:30:46 -05:00
Alejandro Colomar 8fcf6cccff lib/time/day_to_str.[ch]: day_to_str(): Accept a day instead of a date, and rename function
It was always being called with 'day * DAY', so do that internally and
simplify.  This grabs some code from print_day_as_date().

Cc: Tobias Stoeckmann <tobias@stoeckmann.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:30:46 -05:00
Alejandro Colomar 8fee869e9a src/passwd.c: print_status(): Fix typo (bogus use of the comma operator)
Amazing that this triggered no warnings at all.

Fixes: 355ad6a9e0 ("Have a single definition of date_to_str()")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:30:46 -05:00
Alejandro Colomar 82e28ad534 src/: Use DAY_TO_STR() instead of its pattern
Cc: Tobias Stoeckmann <tobias@stoeckmann.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:30:46 -05:00
Alejandro Colomar 19edb06fd2 lib/time/day_to_str.h: DAY_TO_STR(): Add macro
This macro ensures that the buffer is an array, and calculates the size.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:30:46 -05:00
Alejandro Colomar be05c62bd7 lib/, src/, po/: date_to_str(): Move function to header, and make inline
BTW, there's no translatable string in there.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:30:46 -05:00
Alejandro Colomar 88760598f0 src/sulogin.c: Invert logic to reduce indentation
Also, it was checking for >=0 for success, but since that code is for
opening a different tty as stdin, that was bogus.  But since it's
guaranteed to be either 0 or -1, this commit doesn't add any code to
make sure it's 0 (i.e., we could say !=0 instead of ==-1).  That's more
appropriate for a different commit.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:16:15 -05:00
Alejandro Colomar efd169e010 lib/, src/: Use int main(void) where appropriate
Remove /*ARGSUSED*/ comments.  Instead, use appropriate declarators for
main().  ISO C allows using int main(void) if the parameters are going
to be unused.

Also, do some cosmetic changes in the uses of argc and argv, to show
where they are used.

And use *argv[], instead of **argv.  Array notation is friendlier, IMO.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:16:15 -05:00
Alejandro Colomar da440b536c lib/: Clean up after previous removal of dead code
Just cosmetic changes.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:12:51 -05:00
Alejandro Colomar 33825ab57d lib/, src/: Remove all code wrapped in defined(USE_NIS)
I don't find any way to enable USE_NIS, so it looks like it's all
dead code.  Bury it.

Closes: <https://github.com/shadow-maint/shadow/issues/909>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:12:51 -05:00
Alejandro Colomar ae3d71fb94 src/passwd.c: Don't print the program name twice in a log entry
OPENLOG() already sets the program name as the prefix.

This resulted in entries like:

$ journalctl 2>/dev/null | grep passwd
Mar 03 01:09:47 debian passwd[140744]: passwd: can't view or modify password information for root

Fixes: 8e167d28af ("[svn-upgrade] Integrating new upstream version, shadow (4.0.8)")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-14 16:01:32 -05:00
ed neville 4959cd10ae Noting copy_symlink behaviour
Mention that symlinks are modified when they prefix the skel directory.

Closes #933
2024-03-14 15:53:33 -05:00
Alejandro Colomar a3cae72faa share/containers/, .github/workflows/: Don't make(1) twice
It was being done so that the second one prints errors without races.
However, the same thing can be achieved by passing -Orecurse to make(1).

And this makes the logs even more readable, since there's no racy output
at all.

Fixes: 97f79e3b27 ("CI: Make build logs more readable")
Link: <https://github.com/shadow-maint/shadow/pull/702>
Link: <https://github.com/nginx/unit/pull/1123>
Acked-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Andrew Clayton <a.clayton@nginx.com>
Cc: Konstantin Pavlov <thresh@nginx.com>
Cc: Dylan Arbour <https://github.com/arbourd>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-13 11:05:36 -05:00
Alejandro Colomar 26deef6945 lib/idmapping.c: get_map_ranges(): Merge two input checks into a simpler one
Previously, we were performing the following two checks:

-       if (ranges != ((argc + 2) / 3)) {
-       if ((ranges * 3) > argc) {

Let's draw a table of the possible input that would pass the first check:

argc:	0 1 2 3 4 5 6 7 8 9
rng:	0 1 1 1 2 2 2 3 3 3
a+2/3*3:0 3 3 3 6 6 6 9 9 9	<-- this is  roundup(argc, 3);
a+2/3:	0 1 1 1 2 2 2 3 3 3	<-- this is  roundup(argc, 3) / 3;
rng*3:	0 3 3 3 6 6 6 9 9 9

From those, let's extract those that would also pass the second check:

argc:	0     3     6     9
rng:	0     1     2     3
rng*3:	0     3     6     9

We can see that there's a simple check for this input:

+       if (ranges * 3 != argc) {

As a sanity check, let's draw a table of the acceptable input with that
check:

rng:	0     1     2     3
rng*3:	0     3     6     9
argc:	0     3     6     9

Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-13 10:55:00 -05:00
Skyler Ferrante d2f2c1877a Adding checks for fd omission
Adding function check_fds to new file fd.c. The function check_fds
should be called in every setuid/setgid program.

Co-developed-by: Alejandro Colomar <alx@kernel.org>
2024-03-10 19:56:40 -05:00
Alejandro Colomar b76fc2947f tests/unit/test_zustr2stp.c: Test ZUSTR2STP()
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-10 19:55:39 -05:00
Alejandro Colomar ffb3992467 lib/string/zustr2stp.[ch]: Remove zustr2stp(); keep ZUSTR2STP()
The function should never be used; it's always used via its wrapper
macro.  To simplify, and reduce chances of confusion: remove the
function, and implement the macro directly in terms of
stpcpy(mempcpy(strnlen())).

Update the documentation, and improve the example, which was rather
confusing.

Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-10 19:55:39 -05:00
Serge Hallyn ba43b49a52 configure.ac: Release 4.15.0
Signed-off-by: Serge Hallyn <serge@hallyn.com>
4.15.0
2024-03-08 16:04:59 -06:00
Alejandro Colomar 89c4da43cb src/vipw.c: Use string literals to initialize 'Prog'
This avoids using argv[0], which is controlled by the user,
and might inject arbitrary text in stderr and the logs.

Link: <https://github.com/shadow-maint/shadow/issues/959>
Link: <https://github.com/shadow-maint/shadow/pull/960>
Cc: "Skyler Ferrante (RIT Student)" <sjf5462@rit.edu>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Karel Zak <kzak@redhat.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Brauner <christian@brauner.io>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-08 10:24:15 -06:00
Alejandro Colomar 0ab893a734 src/vipw.c: Reverse logic and variable name
Since we're checking for "vigr", it makes more sense to name the
variable accordingly.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-08 10:24:15 -06:00
Skyler Ferrante e6c2e43937 Hardcoding Prog to known value
See #959. We now set Prog (program name) based on hardcoded value instead
of argv[0]. This is to help prevent escape sequence injection.
2024-03-07 22:23:04 +01:00
Alejandro Colomar d13844408c share/containers/: trap(1) to see the cmocka logs
Reviewed-by: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-04 01:43:25 +01:00
Alejandro Colomar e59a39663d share/containers/: Specify one argument per line
Reviewed-by: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-04 01:43:25 +01:00
Alejandro Colomar a14936cf2e .github/workflows/runner.yml: trap(1) to see the testsuite log
Otherwise, 'cat testsuite.log' isn't run, since 'set -e' aborts the
script earlier.

Reviewed-by: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-03-04 01:43:25 +01:00
Serge Hallyn 959343fe79 configure.ac: release 4.15.0-rc3
Signed-off-by: Serge Hallyn <serge@hallyn.com>
4.15.0-rc3
2024-02-29 19:51:37 -06:00
Alejandro Colomar 1af6b68cbe lib/utmp.c: Use the appropriate autotools macros for struct utmpx
Recently, we started using utmpx instead of utmp, and we updated
<./configure.ac> to do the checks for 'struct utmpx' instead of
'struct utmp'.  However, I forgot to update the preprocessor
conditionals accordingly.

Fixes: 64bcb54fa9 ("lib/, src/, configure.ac: Use utmpx instead of utmp")
Link: <https://github.com/shadow-maint/shadow/pull/954>
Cc: Firas Khalil Khana <firasuke@gmail.com>
Cc: "A. Wilfox" <https://github.com/awilfox>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-21 15:43:25 +01:00
Alejandro Colomar 2806b827d8 lib/utmp.c: Use defined() instead of #if[n]def
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-21 15:43:25 +01:00
Alejandro Colomar 7e94a2f484 lib/utmp.c: Remove #endif comments
Indentation makes it clear which is which.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-21 15:43:25 +01:00
Alejandro Colomar e5815acf37 lib/utmp.c: Merge preprocessor conditionals
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-21 15:43:25 +01:00
Alejandro Colomar f4ea04b728 lib/utmp.c: Indent nested preprocessor conditionals
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-21 15:43:25 +01:00
Alejandro Colomar 5ff6edf9f2 lib/utmp.c: Replace UT_LINESIZE by a NITEMS() calculation
A difference between 'struct utmp' and 'struct utmpx' is that
the former uses UT_LINESIZE for the size of its array members,
while the latter doesn't have a standard variable to get its
size.  Therefore, we need to get the number of elements in
the array with NITEMS().

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-20 18:53:53 +01:00
Alejandro Colomar 544709fad3 lib/sizeof.h: memberof(): Add macro
This macro is useful to get the size of a member of a structure
without having a variable of that type.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-20 18:53:53 +01:00
Alejandro Colomar 8d1f0bcf99 lib/utmp.c: get_session_host(): Reduce scope of variable
This silences a warning about an unused variable.

Tested-by: Firas Khalil Khana <firasuke@gmail.com>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-20 18:53:53 +01:00
Alejandro Colomar 64bcb54fa9 lib/, src/, configure.ac: Use utmpx instead of utmp
utmpx is specified by POSIX as an XSI extension.  That's more portable
than utmp, which is unavailable for example in musl libc.  The manual
page specifies that in Linux (but it probably means in glibc), utmp and
utmpx (and the functions that use them) are identical, so this commit
shouldn't affect glibc systems.

Assume utmpx is always present.

Also, if utmpx is present, POSIX guarantees that some members exist:

-  ut_user
-  ut_id
-  ut_line
-  ut_pid
-  ut_type
-  ut_tv

So, rely on them unconditionally.

Fixes: 170b76cdd1 ("Disable utmpx permanently")
Closes: <https://github.com/shadow-maint/shadow/issues/945>
Reported-by: Firas Khalil Khana <firasuke@gmail.com>
Reported-by: "A. Wilfox" <https://github.com/awilfox>
Tested-by: Firas Khalil Khana <firasuke@gmail.com>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-20 18:53:53 +01:00
Alejandro Colomar 4d139ca466 lib/getdate.y: get_date(): Fix calculation
Instead of adding 1, we should add the value the we stored previously in
the variable.

Fixes: 45c6603cc8 ("[svn-upgrade] Integrating new upstream version, shadow (19990709)")
Closes: <https://github.com/shadow-maint/shadow/issues/939>
Link: <https://github.com/shadow-maint/shadow/pull/942>
Reported-by: Michael Vetter <jubalh@iodoru.org>
Reported-by: Gus Kenion <https://github.com/kenion>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-16 19:58:43 -06:00
Tomas Halman e15aa5a8a6 src/passwd.c: check password length upper limit
The passwd silently truncated the password length to PASS_MAX.
This patch introduces check that prints an error message
and exits the call.

Signed-off-by: Tomas Halman <tomas@halman.net>
2024-02-16 15:46:08 -06:00
Tomas Halman dfb4d8fdf9 src/passwd.c: inconsistent password length limit
The passwd utility had hardcoded limit for password lenght set
to 200 characters. In the agetpass.c is used PASS_MAX for
this purpose.

This patch moves the PASS_MAX definition to common place
and uses it in both places.

Signed-off-by: Tomas Halman <tomas@halman.net>
2024-02-16 15:46:08 -06:00
Serge Hallyn 0259f84583 release 4.15.0-rc2
Signed-off-by: Serge Hallyn <serge@hallyn.com>
4.15.0-rc2
2024-02-15 17:54:19 -06:00
NorwayFun d72d99a810 Update Georgian translation 2024-02-14 15:20:14 -06:00
Alejandro Colomar f22ca217cd lib/chkname.c: is_valid_user_name(): Avoid a cast
By using a temporary vairable, we can remove a cast.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Tobias Stoeckmann <tobias@stoeckmann.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-13 16:13:05 -06:00
Alejandro Colomar ad307ee42a lib/chkname.c: is_valid_user_name(): Remove unnecessary check
If (maxsize == -1), then ((size_t)maxsize == SIZE_MAX).  And no size can
ever be >= SIZE_MAX, so it will never return false if sysconf(3) reports
an unlimited user-name size via returning -1.  Well, to be pedantic,
that disallows a user-name siz of precisely SIZE_MAX bytes when
sysconf(3) returns -1.  However, that's probably a good thing; such a
long user name might trigger Undefined Behavior somewhere else, so be
cautious and disallow it.  I hope nobody will be using the entire
address space for a user name.

The commit that introduced that check missed that this code had always
supported unlimited user-name sizes since it was introduced by Iker in
3b7cc05387 ("lib: replace `USER_NAME_MAX_LENGTH` macro"), and
6be85b0baf ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare
warning") even clarified this in the commit message.

So, while the code in 6a1f45d932 ("lib/chkname.c: Support unlimited
user name lengths") wasn't bad per se, the commit message was incorrect.
What that patch did was adding code for handling EINVAL (or any other
errors that a future kernel might add).

To be more pedantically correct, that commit also allowed (under certain
circumstances, user names of SIZE_MAX bytes, but those were originally
allowed (by accident), and only became disallowed in 403a2e3771
("lib/chkname.c: Take NUL byte into account").  But again, let's
disallow those, just to be cautious.

Link: <https://github.com/shadow-maint/shadow/pull/935>
Link: <https://github.com/shadow-maint/shadow/pull/935#discussion_r1477429492>
See-also: 6be85b0baf ("lib/chkname.c: Use tmp variable to avoid a -Wsign-compare warning")
Fixes: 6a1f45d932 ("lib/chkname.c: Support unlimited user name lengths")
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Tobias Stoeckmann <tobias@stoeckmann.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-02-13 16:13:05 -06:00