strlcpy(3) might not be visible since it is declared in <bsd/string.h>.
This can lead to warnings, like:
fields.c: In function 'change_field':
fields.c:103:17: warning: implicit declaration of function 'strlcpy'; did you mean 'strncpy'? [-Wimplicit-function-declaration]
103 | strlcpy (buf, cp, maxsize);
| ^~~~~~~
| strncpy
../lib/fields.c:103:17: warning: type of 'strlcpy' does not match original declaration [-Wlto-type-mismatch]
103 | strlcpy (buf, cp, maxsize);
| ^
/usr/include/bsd/string.h:44:8: note: return value type mismatch
44 | size_t strlcpy(char *dst, const char *src, size_t siz);
| ^
/usr/include/bsd/string.h:44:8: note: type 'size_t' should match type 'int'
/usr/include/bsd/string.h:44:8: note: 'strlcpy' was previously declared here
/usr/include/bsd/string.h:44:8: note: code may be misoptimized unless '-fno-strict-aliasing' is used
Comparisons if different signedness can result in unexpected results.
Add casts to ensure operants are of the same type.
gettime.c: In function 'gettime':
gettime.c:58:26: warning: comparison of integer expressions of different signedness: 'long long unsigned int' and 'time_t' {aka 'long int'} [-Wsign-compare]
58 | } else if (epoch > fallback) {
| ^
Cast to time_t, since epoch is less than ULONG_MAX at this point.
idmapping.c: In function 'write_mapping':
idmapping.c:202:48: warning: comparison of integer expressions of different signedness: 'int' and 'long unsigned int' [-Wsign-compare]
202 | if ((written <= 0) || (written >= (bufsize - (pos - buf)))) {
| ^~
newgidmap.c: In function ‘main’:
newgidmap.c:178:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
178 | if ((written <= 0) || (written >= sizeof(proc_dir_name))) {
| ^~
newuidmap.c: In function ‘main’:
newuidmap.c:107:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
107 | if ((written <= 0) || (written >= sizeof(proc_dir_name))) {
| ^~
Login timed out message prints only first few bytes when write is immediately followed by exit.
Calling exit from new handler provides enough time to display full message.
gethostbyname(3) was removed in POSIX.1-2008. It has been obsoleted,
and replaced by getaddrinfo(3), which is superior in several ways:
- gethostbyname(3) is not reentrant. There's a GNU extension,
gethostbyname_r(3) which is reentrant, but it's not likely to be
standardized for the following reason. And we don't care too much
about this point either.
- gethostbyname(3) only supports IPv4, but getaddrinfo(3) supports both
IPv4 and IPv6 (and may support other address families in the future).
We don't care about reentrancy, so for keeping the code simple (i.e.,
not touch call site to add code to free(3) an allocated buffer), I added
a static buffer for inet_ntop(3). We could address that in the future,
but I don't think it's worth it.
BTW, we also replace inet_ntoa(3) by inet_ntop(3), as a consequence of
using getaddrinfo(3). inet_ntoa(3) is also marked as deprecated, but
that deprecation seems to have been documented only in the manual page,
and POSIX doesn't mark it as deprecated. The deprecation notice goes
back to when the inet_ntop(3) manual page was added by Sam Varshavchik
to the Linux man-pages in version 1.30 (year 2000).
So, this, apart from updating the code to POSIX.1-2008, is also adding
support for IPv6 :) Although, probably many other parts of the code are
written for IPv4 only, so I wouldn't yet claim support for it.
A few notes:
- I didn't check the return value of inet_ntop(3), since it can't fail
for the given input:
- EAFNOSUPPORT: We only call it with AF_INET and AF_INET6.
- ENOSPC: We calculate the size of the buffer to be wide enough:
MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN) so it always fits.
Cc: Dave Hagewood <admin@arrowweb.com>
Cc: Sam Varshavchik
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
When the caller may not change the room number, work phone, or
home number, then rather than prompting for the new one it will
print the existing one. But due to a typo it printed the full name
in place of each of those.
Fix the fields being printed.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
- Since strncpy(3) is not designed to write strings, but rather
(null-padded) character sequences (a.k.a. unterminated strings), we
had to manually append a '\0'. strlcpy(3) creates strings, so they
are always terminated. This removes dependencies between lines, and
also removes chances of accidents.
- Repurposing strncpy(3) to create strings requires calculating the
location of the terminating null byte, which involves a '-1'
calculation. This is a source of off-by-one bugs. The new code has
no '-1' calculations, so there's almost-zero chance of these bugs.
- strlcpy(3) doesn't padd with null bytes. Padding is relevant when
writing fixed-width buffers to binary files, when interfacing certain
APIs (I believe utmpx requires null padding at lease in some
systems), or when sending them to other processes or through the
network. This is not the case, so padding is effectively ignored.
- strlcpy(3) requires that the input string is really a string;
otherwise it crashes (SIGSEGV). Let's check if the input strings are
really strings:
- lib/fields.c:
- 'cp' was assigned from 'newft', and 'newft' comes from fgets(3).
- lib/gshadow.c:
- strlen(string) is calculated a few lines above.
- libmisc/console.c:
- 'cons' comes from getdef_str, which is a bit cryptic, but seems
to generate strings, I guess.1
- libmisc/date_to_str.c:
- It receives a string literal. :)
- libmisc/utmp.c:
- 'tname' comes from ttyname(3), which returns a string.
- src/su.c:
- 'tmp_name' has been passed to strcmp(3) a few lines above.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
It is Undefined Behavior to declare errno (see NOTES in its manual page).
Instead of using the errno dummy declaration, use one that doesn't need
a comment.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This is shorter to write than 'unsigned long int', so we can collapse
some lines. It is guaranteed by C99.
Link: <https://github.com/shadow-maint/shadow/pull/607>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The buffers have a size of 512 (see xmalloc() above), which is what
snprintf(3) expects.
Link: <https://github.com/shadow-maint/shadow/pull/607>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Previous commits, to keep readability of the diffs, left the code that
was previously wrapped by preprocessor coditionals untouched. Apply
some minor cosmetic changes to merge it in the surrounding code.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
On Linux, utmpx and utmp are identical. However, documentation (manual
pages) covers utmp, and just says about utmpx that it's identical to
utmp. It seems that it's preferred to use utmp, at least by reading the
manual pages.
Moreover, we were defaulting to utmp (utmpx had to be explicitly enabled
at configuration time). So, it seems safer to just make it permanent,
which should not affect default builds.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
When useradd sends its ADD_USER event, it is filling in the id field. This is not yet written to disk. When auditd sees the event and the log format is enriched, auditd tries to lookup the user name but it does not exist. This causes the event to never be resolvable since ausearch relies on the lookup information attached by auditd.
The fix is to not send the id information for any event until after close_files() is called. Just the acct field is all that is
Patch by Steve Grubb (afaik).
Reported at https://bugzilla.redhat.com/show_bug.cgi?id=1713432
In a previous commit, we made USE_TERMIOS unconditionally defined.
Let's just remove it, and remove the condition everywhere.
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The definition for this macro was removed in a previous commit.
Reported-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
getpass(3) is broken in all implementations; in some, more than
others, but somewhat broken in all of them. Check the immediate
previous commit, which added the functions, for more details.
Check also the Linux man-pages commit that marked it as
deprecated, for more details:
7ca189099d73bde954eed2d7fc21732bcc8ddc6b.
Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit?id=7ca189099d73bde954eed2d7fc21732bcc8ddc6b>
Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Allow supplementary groups to be set via the /etc/default/useradd config
file. Allowing an administrator to set additonal groups via the GROUPS
configurable and control the default behaviour of useradd.
Report error if usermod asked for moving homedir and it does not exist.
Signed-off-by: Tomáš Mráz <tm@t8m.info>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Introduced by c6c8130db4
After removing snprintf, the format string should get unescaped once.
Fixes#564
Reporter and patch author: DerMouse (github.com/DerMouse)
free(3) accepts NULL, since the oldest ISO C. I guess the
paranoid code was taking care of prehistoric implementations of
free(3). I've never known of an implementation that doesn't
conform to this, so let's simplify this.
Remove xfree(3), which was effectively an equivalent of free(3).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
useradd does not create the files if they don't exist, but if they exist
it will reset user data even if the data did not exist before creating
a hole and an explicitly zero'd data point resulting (especially for
high UIDs) in a lot of zeros ending up in containers and tarballs.
Allow the compiler to verify the format string against the supplied
arguments.
chage.c:239:51: warning: format not a string literal, format string not checked [-Wformat-nonliteral]
239 | (void) strftime (buf, sizeof buf, format, tp);
| ^~~~~~
It was hard to extend the option specification string passed to
getopt_long as the third argument.
The origian code had a branch with WITH_SELINUX ifdef condition. If
one wants to add one more option char with another ifdef condition
like ENABLE_SUBIDS to the spec, the one must enumerate the specs for
all combinations of the conditions:
* WITH_SELINUX && ENABLE_SUBIDS
* WITH_SELINUX && !ENABLE_SUBIDS
* !WITH_SELINUX && ENABLE_SUBIDS
* !WITH_SELINUX && !ENABLE_SUBIDS
With this change, you can append an option char to the spec.
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
No need to redeclare a variable with the same name and type. Just keep
the one with the biggest scope.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Generating salt value depends on /dev/urandom. But after the
function process_root_flag changed the root directory, It does
not exist.
So, generate salt value before changeing the directory.
Fixes: #514
