Commit Graph

3490 Commits

Author SHA1 Message Date
Serge Hallyn
8acec35d1d man/lastlog: remove wrong use of keyword term
Per https://tdg.docbook.org/tdg/4.5/term, term is a word being
defined in a varlistentry.  The 'high uid' description is not a
varlistentry, so <term> and </term> show up in the processed
manpage.  See debian Bug#1072297.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2024-06-05 15:49:54 +02:00
Alejandro Colomar
69f74dbf8a lib/cast.h: const_cast(): Reimplement with _Generic(3)
This makes it much simpler and portable.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-06-04 09:10:23 +02:00
Iker Pedrosa
4e2453fa9f configure: move cmocka library detection
`PKG_CONFIG` variable needs to be set for `PKG_CHECK_MODULES` to
succeed, but this wasn't happening in Fedora because the first
appearance of `PKG_CHECK_MODULES` was conditionally skipped because this
distribution is compiled without `libbsd` support. Thus, moving the
cmocka library detection before libbsd fixes the problem.

Suggested-by: Lukas Slebodnik <lslebodn@fedoraproject.org>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2024-05-28 19:19:58 -05:00
Serge Hallyn
d0fef040ed tests: add the tests/ subdirectory to dist tarball
This is a first step to helping distributions to use our tests in CI.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2024-05-28 19:18:39 -05:00
Alejandro Colomar
71e28359d1 lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function
All call sites have been replaced by functions from "atoi/a2i.h" and
"atoi/str2i.h" recently.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-27 16:32:09 +02:00
Alejandro Colomar
f3a1e1cf09 src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
It is a simpler call, with more type safety.

A consequence of this change is that the program now accepts numbers in
bases 8 and 16.  That's not a problem here, I think.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-27 16:32:09 +02:00
Alejandro Colomar
fb49de61b7 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
All call sites were replaced by a2i() recently.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-27 16:32:09 +02:00
Alejandro Colomar
895dfd77d2 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
time_t isn't necessarily unsigned (in fact, it's likely to be signed.
Therefore, parse the number as the right type, via a2i(time_t, ...).

Still, reject negative numbers, just to be cautious.  It was done
before (strtoull_noneg()), so it shouldn't be a problem.  (However,
strtoull_noneg() was only introduced recently, and before that we called
strtoull(3), which silently accepted negative values.)

Remove the limitation of ULONG_MAX, which seems arbitrary.  It probably
was written in times where 'time_t' had the same length of 'long', and
this was thus a test that the value didn't overflow 'time_t'.  Such a
test is implicit in the a2i() call, so forget about it.

Unify the error messages into a single one that provides all the info
(except the value of 'fallback').

Link: <cb610d54b4 (r136407772)>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Chris Lamb <lamby@debian.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-27 16:32:09 +02:00
Tianyu Chen
9dddcd29f1 STABLE.md: 4.15.x is now stable 2024-05-22 15:10:03 +02:00
Alejandro Colomar
a6eb312f60 src/login.c: main(): Use login_name_max_size()
Instead of raw sysconf(_SC_LOGIN_NAME_MAX) calls, which was being used
without error handling.

Fixes: 3b7cc05387 ("lib: replace `USER_NAME_MAX_LENGTH` macro")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-21 13:26:41 +02:00
Alejandro Colomar
99df9d746e lib/chkname.[ch]: login_name_max_size(): Add function
It encapsulates some logic that we may want to reuse elsewhere.

Link: <https://github.com/shadow-maint/shadow/pull/989>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-21 13:26:41 +02:00
Alejandro Colomar
27e467a61a lib/chkname.[ch]: Fix includes
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-21 13:26:41 +02:00
Alejandro Colomar
d8e6a8b99b src/usermod.c: update_gshadow(): Add helper function
Keep the while loop in the outer function, and move the iteration code
to this new helper.  This makes it a bit more readable.

Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
adf37cccd0 src/usermod.c: update_group(): Add helper function
Keep the while loop in the outer function, and move the iteration code
to this new helper.  This makes it a bit more readable.

Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
da77a82ecb src/usermod.c: update_gshadow_file(): Reduce scope of local variable
After _every_ iteration, 'changed' is always 'false'.  We don't need to
have it outside of the loop.

See:

$ grepc update_gshadow_file . \
| grep -e changed -e goto -e continue -e break -e free_ngrp -e '{' -e '}' \
| pcre2grep -v -M '{\n\t*}';
{
	bool               changed;
	changed = false;
	while ((sgrp = sgr_next ()) != NULL) {
		if (!was_member && !was_admin && !is_member) {
			continue;
		}
		if (was_admin && lflg) {
			changed = true;
		}
		if (was_member) {
			if ((!Gflg) || is_member) {
				if (lflg) {
					changed = true;
				}
			} else {
				changed = true;
			}
		} else if (is_member) {
			changed = true;
		}
		if (!changed)
			goto free_nsgrp;
		changed = false;
	}
}

This was already true in the commit that introduced the code:

$ git show 45c6603cc:src/usermod.c \
| grepc update_gshadow \
| grep -e changed -e goto -e break -e continue -e '\<if\>' -e '{' -e '}' \
| pcre2grep -v -M '{\n\t*}';
{
	int changed;
	changed = 0;
	while ((sgrp = sgr_next())) {
		 * See if the user was a member of this group
		 * See if the user was an administrator of this group
		 * See if the user specified this group as one of their
		if (!was_member && !was_admin && !is_member)
			continue;
		if (was_admin && lflg) {
			changed = 1;
		}
		if (was_member && (!Gflg || is_member)) {
			if (lflg) {
				changed = 1;
			}
		} else if (was_member && Gflg && !is_member) {
			changed = 1;
		} else if (!was_member && Gflg && is_member) {
			changed = 1;
		}
		if (!changed)
			continue;
		changed = 0;
	}
}

Fixes: 45c6603cc8 ("[svn-upgrade] Integrating new upstream version, shadow (19990709)")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
68d42a8fbe src/usermod.c: update_group_file(): Reduce scope of local variable
After _every_ iteration, 'changed' is always 'false'.  We don't need to
have it outside of the loop.

See:

$ grepc update_group_file . \
| grep -e changed -e goto -e continue -e break -e free_ngrp -e '{' -e '}' \
| pcre2grep -v -M '{\n\t*}';
{
	bool                changed;
	changed = false;
	while ((grp = gr_next ()) != NULL) {
		if (!was_member && !is_member) {
			continue;
		}
		if (was_member) {
			if ((!Gflg) || is_member) {
				if (lflg) {
					changed = true;
				}
			} else {
				changed = true;
			}
		} else if (is_member) {
			changed = true;
		}
		if (!changed)
			goto free_ngrp;
		changed = false;
free_ngrp:
	}
}

This was already true in the commit that introduced the code:

$ git show 45c6603cc:src/usermod.c \
| grepc update_group \
| grep -e changed -e goto -e break -e continue -e '\<if\>' -e '{' -e '}' \
| pcre2grep -v -M '{\n\t*}';
{
	int changed;
	changed = 0;
	while ((grp = gr_next())) {
		 * See if the user specified this group as one of their
		if (!was_member && !is_member)
			continue;
		if (was_member && (!Gflg || is_member)) {
			if (lflg) {
				changed = 1;
			}
		} else if (was_member && Gflg && !is_member) {
			changed = 1;
		} else if (!was_member && Gflg && is_member) {
			changed = 1;
		}
		if (!changed)
			continue;
		changed = 0;
	}
}

Fixes: 45c6603cc8 ("[svn-upgrade] Integrating new upstream version, shadow (19990709)")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
71a3238b79 src/usermod.c: update_gshadow_file(): Fix RESOURCE_LEAK (CWE-772)
Report:
> shadow-4.15.0/src/usermod.c:864:3: alloc_fn: Storage is returned from allocation function "__sgr_dup".
> shadow-4.15.0/src/usermod.c:864:3: var_assign: Assigning: "nsgrp" = storage returned from "__sgr_dup(sgrp)".
> shadow-4.15.0/src/usermod.c:964:1: leaked_storage: Variable "nsgrp" going out of scope leaks the storage it points to.
> 962|                   free (nsgrp);
> 963|           }
> 964|-> }
> 965|   #endif                                /* SHADOWGRP */
> 966|

Link: https://issues.redhat.com/browse/RHEL-35383
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
61964aa06b src/usermod.c: update_group_file(): Fix RESOURCE_LEAK (CWE-772)
Report:
> shadow-4.15.0/src/usermod.c:734:3: alloc_fn: Storage is returned from allocation function "__gr_dup".
> shadow-4.15.0/src/usermod.c:734:3: var_assign: Assigning: "ngrp" = storage returned from "__gr_dup(grp)".
> shadow-4.15.0/src/usermod.c:815:1: leaked_storage: Variable "ngrp" going out of scope leaks the storage it points to.
> 813|                   gr_free(ngrp);
> 814|           }
> 815|-> }
> 816|
> 817|   #ifdef SHADOWGRP

Link: https://issues.redhat.com/browse/RHEL-35383
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
81bc78ec5c src/usermod.c: Rename update_gshadow() => update_gshadow_file()
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
b089a63ab3 src/usermod.c: Rename update_group() => update_group_file()
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
151f14ad69 src/usermod.c: Reduce scope of local variables
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-20 09:37:01 +02:00
Alejandro Colomar
1ee066ae1e src/useradd.c: set_defaults(): Fix FILE* leak
Report:
> shadow-4.15.0/src/useradd.c:575:2: alloc_fn: Storage is returned from allocation function "fdopen".
> shadow-4.15.0/src/useradd.c:575:2: var_assign: Assigning: "ofp" = storage returned from "fdopen(ofd, "w")".
> shadow-4.15.0/src/useradd.c:734:2: leaked_storage: Variable "ofp" going out of scope leaks the storage it points to.
> 732|           }
> 733|
> 734|->         return ret;
> 735|   }
> 736|

Link: <https://issues.redhat.com/browse/RHEL-35383>
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-18 01:15:10 +02:00
Alejandro Colomar
e7d1508e07 src/useradd.c: Add fmkstemp() to fix file-descriptor leak
This function creates a temporary file, and returns a FILE pointer to
it.  This avoids dealing with both a file descriptor and a FILE pointer,
and correctly deallocating the resources on error.

The code before this patch was leaking the file descriptor if fdopen(3)
failed.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-18 01:15:10 +02:00
Alejandro Colomar
a74c4b6ae1 src/useradd.c: De-duplicate code
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-18 01:15:10 +02:00
Alejandro Colomar
701fe4cf1a src/useradd.c: set_defaults(): Do not free(3) the result of asprintf(3) if it failed
See asprintf(3):

RETURN VALUE
     When successful,  these  functions  return  the  number  of  bytes
     printed, just like sprintf(3).  If memory allocation wasn’t possi‐
     ble,  or  some other error occurs, these functions will return -1,
     and the contents of strp are undefined.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-18 01:15:10 +02:00
Alejandro Colomar
37ae8827a0 src/useradd.c: set_defaults(): Rename goto label
This will help add other labels in the following commits.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-18 01:15:10 +02:00
Alejandro Colomar
f8fc6371f6 src/useradd.c: set_defaults(): Fix order of clean-ups
Resources should be freed in the inverse order of the allocation.
This refactor prepares for the following commits, which fix some leaks.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-18 01:15:10 +02:00
Iker Pedrosa
4c16416ebc port: fix OVERRUN (CWE-119)
```
shadow-4.15.0/lib/port.c:154:2: alias: Assigning: "port.pt_names" = "ttys". "port.pt_names" now points to element 0 of "ttys" (which consists of 65 8-byte elements).
shadow-4.15.0/lib/port.c:155:2: cond_const: Checking "j < 64" implies that "j" is 64 on the false branch.
shadow-4.15.0/lib/port.c:175:2: overrun-local: Overrunning array of 65 8-byte elements at element index 65 (byte offset 527) by dereferencing pointer "port.pt_names + (j + 1)".
173|           *cp = '\0';
174|           cp++;
175|->         port.pt_names[j + 1] = NULL;
176|
177|           /*
```

Resolves: https://issues.redhat.com/browse/RHEL-35383

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
2024-05-17 16:08:26 +02:00
Alejandro Colomar
0066743c49 lib/getrange.c: getrange(): Report an error when min>max
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-17 15:40:03 +02:00
Alejandro Colomar
29f135777e lib/getrange.c: getrange(): Add missing cast
isdigit(3) requires a cast if the argument is of type 'char'.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-17 15:40:03 +02:00
Alejandro Colomar
34f431f607 lib/getrange.c: getrange(): Add const to pointer
Now that we have const-generic macros, we can use a const pointer.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-17 15:40:03 +02:00
Alejandro Colomar
040ba6a853 lib/getrange.c: getrange(): Use a2ul() instead of strtoul_noneg()
It simplifies the error checking.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-17 15:40:03 +02:00
Alejandro Colomar
b0498564b2 lib/atoi/a2i.[ch]: Add const-generic macros
These overloaded macros allow passing either a const or a non-const
endp, and will call the appropriate function.  This kind of const
overloading has prior art in C23's string functions, such as memchr(3).

Martin suggested using an artificial function pointer in _Generic(3); it
allows switching on various types at the same time.

Also add a comment referring to liba2i's PDF manual for documentation.

Link: <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf#subsubsection.7.26.5.2>
Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114731>
Link: <http://www.alejandro-colomar.es/share/dist/liba2i/git/HEAD/liba2i-HEAD.pdf>
Co-developed-by: Martin Uecker <muecker@gwdg.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-17 15:40:03 +02:00
Alejandro Colomar
26c9dd3715 lib/alloc.h: Reimplement [X]REALLOC[F]() macros with _Generic(3)
Instead of GNU builtins and extensions, these macros can be implemented
with C11's _Generic(3), and the result is much simpler (and safer, since
it's now an error, not just a warning).

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-15 12:08:00 +02:00
Frans Spiesschaert
18ecf3987e updated Dutch translation 2024-05-09 14:25:19 +02:00
Iker Pedrosa
9b3889696b man: update translations for username length
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
2024-05-09 10:56:27 +02:00
Iker Pedrosa
03c31bef87 man: update username length
Fixes: 6a1f45d932 ("lib/chkname.c: Support unlimited user name lengths")
Related-To: https://github.com/shadow-maint/shadow/pull/986

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
2024-05-09 10:56:27 +02:00
Alejandro Colomar
98aefe8772 lib/, src/: Rename some local variables
'endptr' is appropriate internally in strtol(3) because it's a pointer
to 'end', and 'end' itself is a pointer to one-after-the-last character
of the numeric string.  In other words,

	endptr == &end

However, naming the pointer whose address we pass to strtol(3)'s
'endptr' feels wrong, and causes me trouble while parsing the code; I
need to double check the number of dereferences, because something feels
wrong in my head.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
f40bd94856 lib/getrange.c: getrange(): Use goto to deduplicate code
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
7af7361fd6 lib/getrange.c: getrange(): Return early
It's doesn't make much sense to break from a switch() just to return.
Let's return early, to simplify.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
bbb2735cc0 lib/getrange.c: getrange(): Return early to reduce indentation
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
d7ab811a36 lib/getrange.c: getrange(): Don't else after return
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
62a4daa2cd lib/getrange.c: getrange(): Return early to remove an else
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
8d8062c770 lib/getrange.c: getrange(): Remove temporary variable
This means we set the pointees on error, which we didn't do before, but
since we return -1 on error and ignore (don't use) the pointees at call
site, that's fine.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
38a0b0a610 lib/getrange.c: getrange(): Small refactor
All 3 non-error paths in the second part resulted in *has_min = true.
Set in once before the switch(), to simplify.

This means we set this variable on error, which we didn't do before,
but since we return -1 on error and ignore (don't use) the pointees at
call site, that's fine.

Also, move a couple of *has_max = true statements to before a comment,
in preparation for future commits.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Alejandro Colomar
6bf5d6d4f3 lib/getrange.c: getrange(): Small refactor
Set *has_{min,max} = false at the begining, so we only need to set them
to true later.

This means we set these variables on error, which we didn't do before,
but since we return -1 on error and ignore (don't use) the pointees at
call site, that's fine.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2024-05-04 17:22:57 -05:00
Iker Pedrosa
dbd3527c03 share/containers: update build flags for fedora 40
libpam is enabled to provide `passwd` binary from this package, as there
are several password quality checks that are enabled through a PAM
module. Same reason to disable account-tools-setuid.

sssd is disabled because `files provider` has been removed in sssd, and
the underlying functionality in shadow isn't needed anymore.

libcrack dependency was disabled some time ago, but the upstream repo
wasn't updated. Doing it now.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
2024-05-02 10:45:03 +02:00
Iker Pedrosa
dbf3b1ad51 share/containers: sort configuration options
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
2024-05-02 10:45:03 +02:00
Iker Pedrosa
b8f17f9c29 share/containers: fix indentation in fedora
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
2024-05-02 10:45:03 +02:00
Serge Hallyn
2e01b9d7d2 newuidmap and newgidmap manpages: fix fd description
The manpages for newuidmap and newgidmap had a typo "[pid[" instead
of "[pid]".  They were also unclear about what the /proc/pid fd should
be.  Fix both.

Closes #977

Reported-by: igo95862@yandex.ru
Signed-off-by: Serge Hallyn <serge@hallyn.com>
2024-04-10 09:18:40 +02:00