The list_match function handles EXCEPT entries through recursive
calls. It calls itself with NULL, which was then passed to strtok so
parsing continued at current position.
Replacing strtok with strsep, this means that EXCEPT entries never
match, because strsep(NULL, ...) always returns NULL, i.e. the
code treats everything after EXCEPT as non-existing.
Fix this by passing current list pointer to recursive call.
Fixes: 90afe61003 (2024-07-04; "lib/, src/: Use strsep(3) instead of strtok(3)")
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The function user_match actually modifies the string passed as its
first argument, so use char * instead of const char *.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Except for the added (and sorted) includes, the removal of redundant
parentheses, a few cases that have been refactored for readability, and
a couple of non-string cases that I've left out of the change, this
patch can be approximated with the following semantic patch:
$ cat ~/tmp/spatch/streq.sp
@@
expression s;
@@
- '\0' == *s
+ streq(s, "")
@@
expression s;
@@
- '\0' == s[0]
+ streq(s, "")
@@
expression s;
@@
- *s == '\0'
+ streq(s, "")
@@
expression s;
@@
- s[0] == '\0'
+ streq(s, "")
$ find contrib/ lib* src/ -type f \
| xargs spatch --in-place --sp-file ~/tmp/spatch/streq.sp;
Signed-off-by: Alejandro Colomar <alx@kernel.org>
list cannot be NULL in the first iteration, so we don't need a do-while.
Just in case it's not obvious: we know it's not NULL in the first
iteration because right above, in line 772, we've already dereferenced
it.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
strsep(3) is stateless, and so is easier to reason about.
It also has a slight difference: strtok(3) jumps over empty fields,
while strsep(3) respects them as empty fields. In most of the cases
where we were using strtok(3), it makes more sense to respect empty
fields, and this commit probably silently fixes a few bugs.
In other cases (most notably filesystem paths), contiguous delimiters
("//") should be collapsed, so strtok(3) still makes more sense there.
This commit doesn't replace such strtok(3) calls.
While at this, remove some useless variables used by these calls, and
reduce the scope of others.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
When running groupadd or groupmod with the -U|--user option, also update
the group shadow database if it is used.
Fixes: 342c934a (2020-08-09, "add -U option to groupadd and groupmod")
Closes: <https://github.com/shadow-maint/shadow/issues/1124>
Except for the added (and sorted) includes, and the removal of redundant
parentheses, and one special case, this patch can be approximated with
the following semantic patch:
$ cat ~/tmp/spatch/strneq.sp;
@@
expression a, b;
@@
- strcmp(a, b) != 0
+ !streq(a, b)
@@
expression a, b;
@@
- 0 != strcmp(a, b)
+ !streq(a, b)
$ find contrib/ lib* src/ -type f \
| xargs spatch --sp-file ~/tmp/spatch/strneq.sp --in-place;
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Except for the added (and sorted) includes, and the removal of redundant
parentheses, this patch can be approximated with the following semantic
patch:
$ cat ~/tmp/spatch/streq.sp;
@@
expression a, b;
@@
- strcmp(a, b) == 0
+ streq(a, b)
@@
expression a, b;
@@
- 0 == strcmp(a, b)
+ streq(a, b)
@@
expression a, b;
@@
- !strcmp(a, b)
+ streq(a, b)
$ find contrib/ lib* src/ -type f \
| xargs spatch --sp-file ~/tmp/spatch/streq.sp --in-place;
$ git restore lib/string/strcmp/streq.h;
Signed-off-by: Alejandro Colomar <alx@kernel.org>
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>
