Re: [hackers] [ubase][PATCH 1/4] su: simplify logic
On Thu Mar 7, 2024 at 1:19 PM EST, Roberto E. Vargas Caballero wrote: > I think it makes it simpler while keeping the correct behaviour that I broke. Looks good to me!
Re: [hackers] [ubase][PATCH 1/4] su: simplify logic
On Wed Mar 6, 2024 at 4:14 AM EST, Roberto E. Vargas Caballero wrote: > Hi, > > > On Mon, Feb 12, 2024 at 04:25:49PM -0500, neeshy wrote: > > Inline dologin, and simplify common code > > I have applied the 4 patches with a minor modification to the 1st > one. > > Kind regards, > Roberto Vargas. Hello, It seems that the modifications you made break the use case where su is called without a username. It would normally default to the root user, but now it invokes usage() instead. My original patch worked as intended. Could you rebase using the original patch instead? Thank you.
[hackers] [ubase][PATCH 3/4] su: check $SHELL for validity
If $SHELL isn't defined in the environment, the call to execve will
fail when -p is specified. Fallback to the user's login shell if $SHELL
is invalid.
---
su.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/su.c b/su.c
index 812aa43..4fc3c04 100644
--- a/su.c
+++ b/su.c
@@ -28,7 +28,7 @@ int
main(int argc, char *argv[])
{
char *usr = "root", *pass;
- char *shell, *term;
+ char *shell, *envshell, *term;
struct passwd *pw;
char *newargv[3];
uid_t uid;
@@ -90,7 +90,9 @@ main(int argc, char *argv[])
newargv[2] = NULL;
} else {
if (pflag) {
- shell = getenv("SHELL");
+ envshell = getenv("SHELL");
+ if (envshell && envshell[0] != '\0')
+ shell = envshell;
} else {
setenv("HOME", pw->pw_dir, 1);
setenv("SHELL", shell, 1);
--
2.43.1
[hackers] [ubase][PATCH 1/4] su: simplify logic
Inline dologin, and simplify common code
---
su.c | 69
1 file changed, 28 insertions(+), 41 deletions(-)
diff --git a/su.c b/su.c
index 329238f..bc7a94f 100644
--- a/su.c
+++ b/su.c
@@ -18,28 +18,6 @@ extern char **environ;
static int lflag = 0;
static int pflag = 0;
-static int
-dologin(struct passwd *pw)
-{
- char *shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
- char *term = getenv("TERM");
- clearenv();
- setenv("HOME", pw->pw_dir, 1);
- setenv("SHELL", shell, 1);
- setenv("USER", pw->pw_name, 1);
- setenv("LOGNAME", pw->pw_name, 1);
- setenv("TERM", term ? term : "linux", 1);
- if (strcmp(pw->pw_name, "root") == 0)
- setenv("PATH", ENV_SUPATH, 1);
- else
- setenv("PATH", ENV_PATH, 1);
- if (chdir(pw->pw_dir) < 0)
- eprintf("chdir %s:", pw->pw_dir);
- execlp(shell, shell, "-l", NULL);
- weprintf("execlp %s:", shell);
- return (errno == ENOENT) ? 127 : 126;
-}
-
static void
usage(void)
{
@@ -50,9 +28,9 @@ int
main(int argc, char *argv[])
{
char *usr = "root", *pass;
- char *shell;
+ char *shell, *term;
struct passwd *pw;
- char *newargv[2];
+ char *newargv[3];
uid_t uid;
ARGBEGIN {
@@ -66,11 +44,9 @@ main(int argc, char *argv[])
usage();
} ARGEND;
- if (argc < 1)
- ;
- else if (argc == 1)
+ if (argc == 1)
usr = argv[0];
- else
+ else if (argc > 1)
usage();
errno = 0;
@@ -98,13 +74,26 @@ main(int argc, char *argv[])
if (setuid(pw->pw_uid) < 0)
eprintf("setuid:");
+ shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
if (lflag) {
- return dologin(pw);
+ newargv[0] = shell;
+ newargv[1] = "-l";
+ newargv[2] = NULL;
+ term = getenv("TERM");
+ clearenv();
+ setenv("HOME", pw->pw_dir, 1);
+ setenv("SHELL", shell, 1);
+ setenv("USER", pw->pw_name, 1);
+ setenv("LOGNAME", pw->pw_name, 1);
+ setenv("TERM", term ? term : "linux", 1);
+ if (chdir(pw->pw_dir) < 0)
+ eprintf("chdir %s:", pw->pw_dir);
} else {
- shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
newargv[0] = shell;
newargv[1] = NULL;
- if (!pflag) {
+ if (pflag) {
+ shell = getenv("SHELL");
+ } else {
setenv("HOME", pw->pw_dir, 1);
setenv("SHELL", shell, 1);
if (strcmp(pw->pw_name, "root") != 0) {
@@ -112,14 +101,12 @@ main(int argc, char *argv[])
setenv("LOGNAME", pw->pw_name, 1);
}
}
- if (strcmp(pw->pw_name, "root") == 0)
- setenv("PATH", ENV_SUPATH, 1);
- else
- setenv("PATH", ENV_PATH, 1);
- execve(pflag ? getenv("SHELL") : shell,
- newargv, environ);
- weprintf("execve %s:", shell);
- return (errno == ENOENT) ? 127 : 126;
}
- return 0;
-}
\ No newline at end of file
+ if (strcmp(pw->pw_name, "root") == 0)
+ setenv("PATH", ENV_SUPATH, 1);
+ else
+ setenv("PATH", ENV_PATH, 1);
+ execve(shell, newargv, environ);
+ weprintf("execve %s:", shell);
+ return (errno == ENOENT) ? 127 : 126;
+}
--
2.43.1
[hackers] [ubase][PATCH 4/4] su: don't set $PATH
Just /bin is too restrictive, and login shells set the path anyway via
the default profile. Also, carrying the path over for non-login shells
conforms to the behavior of util-linux's su.
---
config.def.h | 1 -
su.c | 5 -
2 files changed, 6 deletions(-)
diff --git a/config.def.h b/config.def.h
index 577833e..257cfac 100644
--- a/config.def.h
+++ b/config.def.h
@@ -1,6 +1,5 @@
/* See LICENSE file for copyright and license details. */
-#define ENV_SUPATH "/bin"
#define ENV_PATH "/bin"
#define PW_CIPHER "$6$" /* SHA-512 */
#undef UTMP_PATH
diff --git a/su.c b/su.c
index 4fc3c04..4fc72e8 100644
--- a/su.c
+++ b/su.c
@@ -9,7 +9,6 @@
#include
#include
-#include "config.h"
#include "passwd.h"
#include "util.h"
@@ -104,10 +103,6 @@ main(int argc, char *argv[])
newargv[0] = shell;
newargv[1] = NULL;
}
- if (strcmp(pw->pw_name, "root") == 0)
- setenv("PATH", ENV_SUPATH, 1);
- else
- setenv("PATH", ENV_PATH, 1);
execve(shell, newargv, environ);
weprintf("execve %s:", shell);
return (errno == ENOENT) ? 127 : 126;
--
2.43.1
[hackers] [ubase][PATCH 3/4] su: check $SHELL for validity
If $SHELL isn't defined in the environment, the call to execve will
fail when -p is specified. Fallback to the user's login shell if $SHELL
is invalid.
---
su.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/su.c b/su.c
index 812aa43..4fc3c04 100644
--- a/su.c
+++ b/su.c
@@ -28,7 +28,7 @@ int
main(int argc, char *argv[])
{
char *usr = "root", *pass;
- char *shell, *term;
+ char *shell, *envshell, *term;
struct passwd *pw;
char *newargv[3];
uid_t uid;
@@ -90,7 +90,9 @@ main(int argc, char *argv[])
newargv[2] = NULL;
} else {
if (pflag) {
- shell = getenv("SHELL");
+ envshell = getenv("SHELL");
+ if (envshell && envshell[0] != '\0')
+ shell = envshell;
} else {
setenv("HOME", pw->pw_dir, 1);
setenv("SHELL", shell, 1);
--
2.43.1
[hackers] [ubase][PATCH 1/4] su: simplify logic
Inline dologin, and simplify common code
---
su.c | 69
1 file changed, 28 insertions(+), 41 deletions(-)
diff --git a/su.c b/su.c
index 329238f..bc7a94f 100644
--- a/su.c
+++ b/su.c
@@ -18,28 +18,6 @@ extern char **environ;
static int lflag = 0;
static int pflag = 0;
-static int
-dologin(struct passwd *pw)
-{
- char *shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
- char *term = getenv("TERM");
- clearenv();
- setenv("HOME", pw->pw_dir, 1);
- setenv("SHELL", shell, 1);
- setenv("USER", pw->pw_name, 1);
- setenv("LOGNAME", pw->pw_name, 1);
- setenv("TERM", term ? term : "linux", 1);
- if (strcmp(pw->pw_name, "root") == 0)
- setenv("PATH", ENV_SUPATH, 1);
- else
- setenv("PATH", ENV_PATH, 1);
- if (chdir(pw->pw_dir) < 0)
- eprintf("chdir %s:", pw->pw_dir);
- execlp(shell, shell, "-l", NULL);
- weprintf("execlp %s:", shell);
- return (errno == ENOENT) ? 127 : 126;
-}
-
static void
usage(void)
{
@@ -50,9 +28,9 @@ int
main(int argc, char *argv[])
{
char *usr = "root", *pass;
- char *shell;
+ char *shell, *term;
struct passwd *pw;
- char *newargv[2];
+ char *newargv[3];
uid_t uid;
ARGBEGIN {
@@ -66,11 +44,9 @@ main(int argc, char *argv[])
usage();
} ARGEND;
- if (argc < 1)
- ;
- else if (argc == 1)
+ if (argc == 1)
usr = argv[0];
- else
+ else if (argc > 1)
usage();
errno = 0;
@@ -98,13 +74,26 @@ main(int argc, char *argv[])
if (setuid(pw->pw_uid) < 0)
eprintf("setuid:");
+ shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
if (lflag) {
- return dologin(pw);
+ newargv[0] = shell;
+ newargv[1] = "-l";
+ newargv[2] = NULL;
+ term = getenv("TERM");
+ clearenv();
+ setenv("HOME", pw->pw_dir, 1);
+ setenv("SHELL", shell, 1);
+ setenv("USER", pw->pw_name, 1);
+ setenv("LOGNAME", pw->pw_name, 1);
+ setenv("TERM", term ? term : "linux", 1);
+ if (chdir(pw->pw_dir) < 0)
+ eprintf("chdir %s:", pw->pw_dir);
} else {
- shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
newargv[0] = shell;
newargv[1] = NULL;
- if (!pflag) {
+ if (pflag) {
+ shell = getenv("SHELL");
+ } else {
setenv("HOME", pw->pw_dir, 1);
setenv("SHELL", shell, 1);
if (strcmp(pw->pw_name, "root") != 0) {
@@ -112,14 +101,12 @@ main(int argc, char *argv[])
setenv("LOGNAME", pw->pw_name, 1);
}
}
- if (strcmp(pw->pw_name, "root") == 0)
- setenv("PATH", ENV_SUPATH, 1);
- else
- setenv("PATH", ENV_PATH, 1);
- execve(pflag ? getenv("SHELL") : shell,
- newargv, environ);
- weprintf("execve %s:", shell);
- return (errno == ENOENT) ? 127 : 126;
}
- return 0;
-}
\ No newline at end of file
+ if (strcmp(pw->pw_name, "root") == 0)
+ setenv("PATH", ENV_SUPATH, 1);
+ else
+ setenv("PATH", ENV_PATH, 1);
+ execve(shell, newargv, environ);
+ weprintf("execve %s:", shell);
+ return (errno == ENOENT) ? 127 : 126;
+}
--
2.43.0
[hackers] [ubase][PATCH 2/4] su: fix setting argv0
argv0 was being set to the user's login shell even when -p was
specified. Only populate newargv once the shell is properly determined.
---
su.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/su.c b/su.c
index bc7a94f..812aa43 100644
--- a/su.c
+++ b/su.c
@@ -76,9 +76,6 @@ main(int argc, char *argv[])
shell = pw->pw_shell[0] == '\0' ? "/bin/sh" : pw->pw_shell;
if (lflag) {
- newargv[0] = shell;
- newargv[1] = "-l";
- newargv[2] = NULL;
term = getenv("TERM");
clearenv();
setenv("HOME", pw->pw_dir, 1);
@@ -88,9 +85,10 @@ main(int argc, char *argv[])
setenv("TERM", term ? term : "linux", 1);
if (chdir(pw->pw_dir) < 0)
eprintf("chdir %s:", pw->pw_dir);
- } else {
newargv[0] = shell;
- newargv[1] = NULL;
+ newargv[1] = "-l";
+ newargv[2] = NULL;
+ } else {
if (pflag) {
shell = getenv("SHELL");
} else {
@@ -101,6 +99,8 @@ main(int argc, char *argv[])
setenv("LOGNAME", pw->pw_name, 1);
}
}
+ newargv[0] = shell;
+ newargv[1] = NULL;
}
if (strcmp(pw->pw_name, "root") == 0)
setenv("PATH", ENV_SUPATH, 1);
--
2.43.0
