From 28aed997f871faefc5168eed5731c326ec218edd Mon Sep 17 00:00:00 2001 From: Marcin Nowakowski Date: Tue, 25 Jun 2024 09:45:33 +0200 Subject: [PATCH] Fix coverity unbound buffer issues 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 verifing size of passed arguments for users and groups names. This will not fix the coverity reports, but the change causes that they are irrelevant and could be ignored. --- src/chfn.c | 5 +++++ src/chsh.c | 6 ++++++ src/newgrp.c | 13 +++++++++++++ src/passwd.c | 5 +++++ 4 files changed, 29 insertions(+) diff --git a/src/chfn.c b/src/chfn.c index 1872b2df4..68ee53400 100644 --- a/src/chfn.c +++ b/src/chfn.c @@ -34,6 +34,7 @@ #include "string/sprintf/snprintf.h" #include "string/strcpy/strtcpy.h" #include "string/strdup/xstrdup.h" +#include "chkname.h" /* @@ -643,6 +644,10 @@ int main (int argc, char **argv) * name, or the name getlogin() returns. */ if (optind < argc) { + if (!is_valid_user_name (argv[optind])) { + fprintf (stderr, _("%s: Provided user name is not a valid name\n"), Prog); + fail_exit (E_NOPERM); + } user = argv[optind]; pw = xgetpwnam (user); if (NULL == pw) { diff --git a/src/chsh.c b/src/chsh.c index 4e85678da..0c596997e 100644 --- a/src/chsh.c +++ b/src/chsh.c @@ -32,6 +32,8 @@ #include "shadowlog.h" #include "string/strcpy/strtcpy.h" #include "string/strdup/xstrdup.h" +#include "chkname.h" + #ifndef SHELLS_FILE #define SHELLS_FILE "/etc/shells" @@ -499,6 +501,10 @@ int main (int argc, char **argv) * name, or the name getlogin() returns. */ if (optind < argc) { + if (!is_valid_user_name (argv[optind])) { + fprintf (stderr, _("%s: Provided user name is not a valid name\n"), Prog); + fail_exit (E_NOPERM); + } user = argv[optind]; pw = xgetpwnam (user); if (NULL == pw) { diff --git a/src/newgrp.c b/src/newgrp.c index 11fc6f82a..979901e42 100644 --- a/src/newgrp.c +++ b/src/newgrp.c @@ -27,6 +27,7 @@ #include "shadowlog.h" #include "string/sprintf/snprintf.h" #include "string/strdup/xstrdup.h" +#include "chkname.h" /* @@ -483,6 +484,12 @@ int main (int argc, char **argv) * not "newgrp". */ if ((argc > 0) && (argv[0][0] != '-')) { + if (!is_valid_group_name (argv[0])) { + fprintf ( + stderr, _("%s: provided group is not a valid group name\n"), + Prog); + goto failure; + } group = argv[0]; argc--; argv++; @@ -514,6 +521,12 @@ int main (int argc, char **argv) usage (); goto failure; } else if (argv[0] != NULL) { + if (!is_valid_group_name (argv[0])) { + fprintf ( + stderr, _("%s: provided group is not a valid group name\n"), + Prog); + goto failure; + } group = argv[0]; } else { /* diff --git a/src/passwd.c b/src/passwd.c index 8a46bc0d7..7c6b3a82d 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -36,6 +36,7 @@ #include "string/strcpy/strtcpy.h" #include "string/strdup/xstrdup.h" #include "time/day_to_str.h" +#include "chkname.h" /* @@ -910,6 +911,10 @@ main(int argc, char **argv) } myname = xstrdup (pw->pw_name); if (optind < argc) { + if (!is_valid_user_name (argv[optind])) { + fprintf (stderr, _("%s: Provided user name is not a valid name\n"), Prog); + fail_exit (E_NOPERM); + } name = argv[optind]; } else { name = myname;