> From: Dmitry Bogatov <KAction_AT_gnu.org>
Hi Reiner.
> getpwnam(3) recommends to use $HOME instead of getpwuid()->pw_dir,
> as it allows users to point programs to a different path.
>
> Using getpwuid() also breaks namespaces-related use cases,
> like `unshare -r`.
Thanks for the report!
I'm ok with fixing this issue, but not so much with the actual patch.
First, the overall style of the patch doesn't follow the suckless
coding style and it would have to be rewritten anyway.
> Patch was submitted by Dmitry Bogatov on the Debian bug tracker:
> https://bugs.debian.org/825397
> ---
> surf.c | 43 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/surf.c b/surf.c
> index 23c49bd..46b0ce8 100644
> --- a/surf.c
> +++ b/surf.c
> _AT_@ -287,29 +287,58 @@ buildfile(const char *path)
> return fpath;
> }
>
> +static const char*
> +get_user_homedir(const char *user) {
> + struct passwd *pw = getpwnam(user);
> + if (!pw) {
> + die("Can't get user `%s' home directory.\n", user);
Maybe changing the error message to something like “Can't get user %s
login information.”.
> + }
> + return pw->pw_dir;
> +}
ok
> +
> +static const char*
> +get_current_user_homedir() {
> + const char *homedir;
> + const char *user;
> + struct passwd *pw;
> +
> + homedir = getenv("HOME");
> + if (homedir) {
> + return homedir;
> + }
ok
> +
> + user = getenv("USER");
> + if (user) {
> + return get_user_homedir(user);
> + }
ok
> +
> + pw = getpwuid(getuid());
> + if (!pw) {
> + die("Can't get current user home directory\n");
> + }
Wouldn't it better to use geteuid() there?
> + return pw->pw_dir;
> +}
> +
> char *
> buildpath(const char *path)
> {
> - struct passwd *pw;
> char *apath, *name, *p, *fpath;
>
> if (path[0] == '~') {
> + const char *homedir;
> if (path[1] == '/' || path[1] == '\0') {
> p = (char *)&path[1];
> - pw = getpwuid(getuid());
> + homedir = get_current_user_homedir();
> } else {
> if ((p = strchr(path, '/')))
> name = g_strndup(&path[1], --p -
> path); else
> name = g_strdup(&path[1]);
>
> - if (!(pw = getpwnam(name))) {
> - die("Can't get user %s home
> directory: %s.\n",
> - name, path);
> - }
> + homedir = get_user_homedir(name);
> g_free(name);
> }
> - apath = g_build_filename(pw->pw_dir, p, NULL);
> + apath = g_build_filename(homedir, p, NULL);
> } else {
> apath = g_strdup(path);
> }
ok
If Dmitry is ok to rewrite the patch as per our suggestions, I'd be ok
to apply it. Else I'll do it myself.
Thanks again for the report+patch!
Received on Sun May 29 2016 - 14:42:40 CEST