The reason for that code seems to be some ancient AIX version that
defined a value that was too small (32). We don't support such systems.
In the link below, I found the following comment and code:
/*
* Some AIX versions advertise a too small MAXHOSTNAMELEN value (32).
* Result: long hostnames would be truncated, and connections would be
* dropped because of host name verification failures. Adrian van Bloois
* (A.vanBloois@info.nic.surfnet.nl) figured out what was the problem.
*/
#if (MAXHOSTNAMELEN < 64)
#undef MAXHOSTNAMELEN
#endif
/* In case not defined in <sys/param.h>. */
#ifndef MAXHOSTNAMELEN
#define MAXHOSTNAMELEN 256 /* storage for host name */
#endif
Today's systems seem to be much better regarding this macro. Rely on
them.
Link: <https://sources.debian.org/src/tcp-wrappers/7.6.q-33/workarounds.c/?hl=36#L36>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This fix addresses an issue in is_valid_user_list() where the free
operation was attempted on an address not allocated with malloc(). By
duplicating the pointer with xstrdup(users) into dup, and using dup as
the original pointer, we ensure that only the valid pointer is freed,
avoiding an invalid free operation.
This bug was introduced when changing some code that used strchrnul(3)
to use strsep(3) instead. strsep(3) advances the pointer, unlike the
previous code.
This unconditionally leads to a bug:
- Passing NULL to free(3), if the last field in the
colon-separated-value list is non-empty. This results in a memory
leak.
- Passing a pointer to the null byte ('\0') that terminates the string,
if the last element of the colon-separated-value list is empty. The
most obvious reproducer of such a bogus free(3) call is:
free(strdup("foo:") + 4);
This results in Undefined Behavior, and could result in allocator
data corruption.
Fixes: 16cb664865 (2024-07-01, "lib/, src/: Use strsep(3) instead of its pattern")
Suggested-by: <https://github.com/frostb1ten>
Reported-by: <https://github.com/frostb1ten>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Brauner <christian@brauner.io>
During coverity scan, there are reported four issues
with unbounded source buffer for each usage of input arg
directly with syslog function.
Sample coverity test report for chsh.c file:
1. string_size_argv: argv contains strings with unknown size.
int main (int argc, char **argv)
[...]
4. var_assign_var: Assigning: user = argv[optind]. Both are now tainted.
user = argv[optind];
[...]
CID 5771784: (#1 of 1): Unbounded source buffer (STRING_SIZE)
15. string_size: Passing string user of unknown size to syslog.
SYSLOG ((LOG_INFO, "changed user '%s' shell to '%s'", user, loginsh));
Similar issue is reported three times more:
File: chfn.c, function: main, variable: user
File: passwd.c, function: main, variable: name
File: newgrp.c, function: main, variable: group
This commit is the first approach to fix the reported issues.
The proposed changes add conditions, which verify
the user and group names arguments, including their lengths.
This will not silence the coverity reports, but the change causes
that they are irrelevant and could be ignored.
I forgot to remove the setting of errno when I switched from
strtoul_noneg() to str2ul(). strtoul(3) needs errno for determining
success, but str2ul() does not.
Fixes: f3a1e1cf09 ("src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The mode of the file should be 644, but mkstemp(2) was transforming it
to 600.
To do this, we need a function that accepts a mode parameter. While we
don't need a flags parameter, to avoid confusion with mkostemp(2), let's
add both a flags and a mode parameter.
Link: <https://github.com/shadow-maint/shadow/pull/1080>
Reported-by: kugarocks <kugacola@gmail.com>
Suggested-by: kugarocks <kugacola@gmail.com>
Tested-by: kugarocks <kugacola@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
If localtime_r(3) fails, just print future, as we do in day_to_str().
It should only fail for unrealistic dates, if at all.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
%F is specified by ISO C99. It adds semantic meaning as printing an
ISO 8601 date.
Scripted change:
$ cat ~/tmp/spatch/strftime_F.sp
@@
@@
- "%Y-%m-%d"
+ "%F"
$ find contrib/ lib* src/ -type f \
| xargs spatch --sp-file ~/tmp/spatch/strftime_F.sp --in-place
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This requires changing isspace(3) calls to an explicit accept string,
and I chose " \t\n" for it (as is done in other parts of this project),
which isn't exactly the same, but we probably don't want other
isspace(3) characters in those files, so it should work.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
STRNDUPA() is equivalent to automatic storage allocation (alloca(3))
+ ZUSTR2STP().
The benefits of this refactor are:
- The allocation size is always correct, and needs no comments, since
it's now automatically calculated by the macro.
- STRNDUPA() is probably more familiar, since
- strndupa(3) is a libc function,
- STRNDUPA() is the obvious wrapper that
calculates the size based on the input array.
- We can remove ZUSTR2STP().
Signed-off-by: Alejandro Colomar <alx@kernel.org>
The program was happily ignoring ENOMEM errors.
Fixes: 7f9e196903 ("* NEWS, src/newusers.c, src/Makefile.am: Added support for")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
We are setting `sgrent.sg_adm[1] = NULL;`, so we need 2 elements.
Fixes: 87b56b19fb ("* NEWS, src/groupmems.c, man/groupmems.8.xml: Added support for [...]")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
