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 is reported an issue
with unbounded source buffer for each usage of input arg
directly with syslog function.
This commit is to limit the names of users and groups
passed as input args to a fixed value of 256.
The max length 256 is a typical POSIX login name length.
This should improve also security.
  • Loading branch information
MarcinDigitic authored and alejandro-colomar committed May 30, 2024
1 parent 4e2453f commit bb74110
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 5 deletions.
20 changes: 20 additions & 0 deletions lib/alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,23 @@ xreallocarray(void *p, size_t nmemb, size_t size)
log_get_progname(), strerror(errno));
exit(13);
}

char *
xstrndup(const char *s, size_t max_length) {
size_t length = strlen(s);
if (length > max_length) {
length = max_length;
}

char *p = malloc(length + 1);
if (!p) {
fprintf(log_get_logfd(), _("%s: %s\n"),
log_get_progname(), strerror(errno));
exit(13);
}

strncpy(p, s, length);
p[length] = '\0';

return p;
}
2 changes: 2 additions & 0 deletions lib/alloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ void *xcalloc(size_t nmemb, size_t size);
ATTR_MALLOC(free)
void *xreallocarray(void *p, size_t nmemb, size_t size);

ATTR_MALLOC(free)
char *xstrndup(const char *s, size_t max_length);

inline void *
xmalloc(size_t size)
Expand Down
9 changes: 9 additions & 0 deletions lib/defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,13 @@
#define PASS_MAX BUFSIZ - 1
#endif

/* Maximum length of user name or group name
* This is used only to limit a string length passed to syslog.
* An issue "unbounded source buffer" is reported while doing
* coverity scans, when there is an unlimited string length passed
* to syslog function.
* Typical POSIX login name max length is 256.
*/
#define USER_ENTRY_MAX_LENGTH 256

#endif /* _DEFINES_H_ */
2 changes: 1 addition & 1 deletion src/chfn.c
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ int main (int argc, char **argv)
* name, or the name getlogin() returns.
*/
if (optind < argc) {
user = argv[optind];
user = xstrndup(argv[optind], USER_ENTRY_MAX_LENGTH);
pw = xgetpwnam (user);
if (NULL == pw) {
fprintf (stderr, _("%s: user '%s' does not exist\n"), Prog,
Expand Down
2 changes: 1 addition & 1 deletion src/chsh.c
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ int main (int argc, char **argv)
* name, or the name getlogin() returns.
*/
if (optind < argc) {
user = argv[optind];
user = xstrndup(argv[optind], USER_ENTRY_MAX_LENGTH);
pw = xgetpwnam (user);
if (NULL == pw) {
fprintf (stderr,
Expand Down
4 changes: 2 additions & 2 deletions src/newgrp.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ int main (int argc, char **argv)
* not "newgrp".
*/
if ((argc > 0) && (argv[0][0] != '-')) {
group = argv[0];
group = xstrndup(argv[0], USER_ENTRY_MAX_LENGTH);
argc--;
argv++;
} else {
Expand Down Expand Up @@ -513,7 +513,7 @@ int main (int argc, char **argv)
usage ();
goto failure;
} else if (argv[0] != NULL) {
group = argv[0];
group = xstrndup(argv[0], USER_ENTRY_MAX_LENGTH);
} else {
/*
* get the group file entry for her login group id.
Expand Down
2 changes: 1 addition & 1 deletion src/passwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ int main (int argc, char **argv)
}
myname = xstrdup (pw->pw_name);
if (optind < argc) {
name = argv[optind];
name = xstrndup(argv[optind], USER_ENTRY_MAX_LENGTH);
} else {
name = myname;
}
Expand Down

0 comments on commit bb74110

Please sign in to comment.