Skip to content

Commit

Permalink
Fix coverity unbound buffer issues
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MarcinDigitic authored and mnconlusive committed Oct 14, 2024
1 parent 4a15739 commit 28aed99
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/chfn.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "string/sprintf/snprintf.h"
#include "string/strcpy/strtcpy.h"
#include "string/strdup/xstrdup.h"
#include "chkname.h"


/*
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions src/chsh.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 13 additions & 0 deletions src/newgrp.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "shadowlog.h"
#include "string/sprintf/snprintf.h"
#include "string/strdup/xstrdup.h"
#include "chkname.h"


/*
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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 {
/*
Expand Down
5 changes: 5 additions & 0 deletions src/passwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "string/strcpy/strtcpy.h"
#include "string/strdup/xstrdup.h"
#include "time/day_to_str.h"
#include "chkname.h"


/*
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 28aed99

Please sign in to comment.