e5905c4b ("Added control character check") introduced checking for
control characters but had the logic inverted, so it rejects all
characters that are not control ones.
Cast the character to `unsigned char` before passing to the character
checking functions to avoid UB.
Use strpbrk(3) for the illegal character test and return early.
Both semanage and libsemanage actually set the user's mls range to the
default of the seuser, which makes more sense and removes a bit of code
for usermod and useradd. More fine-grained details must always be set
with some other tool
(semanage) anyway.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The --gid option accepts a group name or id. When a name is provided, it
is resolved to an id by looking up the name in the group database
(/etc/group).
The --prefix option overides the location of the passwd and group
databases. I suspect the --gid option was overlooked when wiring up the
--prefix option.
useradd --gid already respects --prefix; this change makes usermod
behave the same way.
Fixes: b6b2c756c9
Signed-off-by: Mike Gilbert <floppym@gentoo.org>
* src/su.c (check_perms): Do not silently truncate user name.
Reported-by: Paul Eggert <eggert@cs.ucla.edu>
Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
This commit will serve to document why we shouldn't worry about the
truncation in the call to strlcpy(3). Since we have one more byte in
tmptty than in full_tty, truncation will produce a string that is at
least one byte longer than full_tty. Such a string could never compare
equal, so we're actually handling the truncation in a clever way. Maybe
too clever, but that's why I'm documenting it here.
Now, about the simplification itself:
Since we made sure that both full_tty and tmptty are null-terminated, we
can call strcmp(3) instead of strncmp(3). We can also simplify the
return logic avoiding one branch.
Cc: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* libmisc/utmp.c (is_my_tty): Declare the parameter as a char array,
not char *, as it is not necessarily null-terminated.
Avoid a read overrun when reading 'tty', which comes from
'ut_utname'.
Reported-by: Paul Eggert <eggert@cs.ucla.edu>
Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* libmisc/date_to_str.c (date_to_str): Do not crash if gmtime(3)
returns NULL because the timestamp is far in the future.
Reported-by: Paul Eggert <eggert@cs.ucla.edu>
Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/gshadow.c (sgetsgent): Use strcpy(3) not strlcpy(3),
since the string is known to fit.
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/fields.c (change_field): Don't point
before array start; that has undefined behavior.
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/fields.c (change_field): Since we know the string fits,
use strcpy(3) rather than strlcpy(3).
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
gettime.c:25:30: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
/*@observer@*/time_t gettime ()
^
void
shellcheck warns against using echo with flags, as posix sh won't
support it. It suggests using printf, so let's do that.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
bc github...
For some reason, the first test - ONLY on github - seems to not
give the '$ ' prompt expected when you spawn 'su testsuite'.
So just run the first test twice, and ignore the first failure.
It messes with the expected results.
We can do better than this in the expect scripts, but let's
get things running for now.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
write_mapping() will do the following:
openat(proc_dir_fd, map_file, O_WRONLY);
An attacker could create a directory containing a symlink named
"uid_map" pointing to any file owned by root, and thus allow him to
overwrite any root-owned file.
Closes#635
newuidmap and newgidmap currently take an integner pid as
the first argument, determining the process id on which to
act. Accept also "fd:N", where N must be an open file
descriptor to the /proc/pid directory for the process to
act upon. This way, if you
exec 10</proc/99
newuidmap fd:10 100000 0 65536
and pid 99 dies and a new process happens to take pid 99 before
newuidmap happens to do its work, then since newuidmap will use
openat() using fd 10, it won't change the mapping for the new
process.
Example:
// terminal 1:
serge@jerom ~/src/nsexec$ ./nsexec -W -s 0 -S 0 -U
about to unshare with 10000000
Press any key to exec (I am 129176)
// terminal 2:
serge@jerom ~/src/shadow$ exec 10</proc/129176
serge@jerom ~/src/shadow$ sudo chown root src/newuidmap src/newgidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newuidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newgidmap
serge@jerom ~/src/shadow$ ./src/newuidmap fd:10 0 100000 10
serge@jerom ~/src/shadow$ ./src/newgidmap fd:10 0 100000 10
// Terminal 1:
uid=0(root) gid=0(root) groups=0(root)
Signed-off-by: Serge Hallyn <serge@hallyn.com>
We can't use a pointer that was input to realloc(3), nor any pointers
that point to reallocated memory, without making sure that the memory
wasn't moved. If we do, the Behavior is Undefined.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Use of these macros, apart from the benefits mentioned in the commit
that adds the macros, has some other good side effects:
- Consistency in getting the size of the object from sizeof(type),
instead of a mix of sizeof(type) sometimes and sizeof(*p) other
times.
- More readable code: no casts, and no sizeof(), so also shorter lines
that we don't need to cut.
- Consistency in using array allocation calls for allocations of arrays
of objects, even when the object size is 1.
Cc: Valentin V. Bartenev <vbartenev@gmail.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
