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, 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.

diff --git a/src/chfn.c b/src/chfn.c
index 1872b2df4cb33a3d2c820bb807c8b6f45c39fc2d..68ee5340068d596c707c8c750ddc1bdab2840210 100644 (file)
--- 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 4e85678da120f55128d85d0a8463a57712a2b661..d1488c674f89c2a477aa7adece1fe20939eebdc0 100644 (file)
--- 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 (1);
+               }
                user = argv[optind];
                pw = xgetpwnam (user);
                if (NULL == pw) {

diff --git a/src/newgrp.c b/src/newgrp.c
index 11fc6f82a94627f74f677bd7b585e08372e18f8e..979901e4239e22cb3d0cb45fdb9b68e4d301c750 100644 (file)
--- 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 8a46bc0d7e95b8586b584043ff73ec1e56355632..7c6b3a82dc5d5061d2e96081f0a5678ff9b6ca93 100644 (file)
--- 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;