From eebc763bd3fabcaebdace872d07eea89518b8554 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Thu, 13 Mar 2003 20:00:45 +0000 Subject: [PATCH] Use strlc{at,py} for paranoia's sake and exit on overflow. In all cases the strings were either pre-allocated to the correct size of length checks were done before the copy but a little paranoia can go a long way. --- check.c | 36 +++++++++++++++++++++++++----------- env.c | 24 ++++++++++++++---------- find_path.c | 7 ++++--- parse.c | 5 ++--- parse.yacc | 9 ++++----- sudo.c | 19 ++++++++++++------- testsudoers.c | 18 ++++++++++++------ 7 files changed, 73 insertions(+), 45 deletions(-) diff --git a/check.c b/check.c index a86c138ff..0e2464510 100644 --- a/check.c +++ b/check.c @@ -174,9 +174,9 @@ expand_prompt(old_prompt, user, host) char *user; char *host; { - size_t len; + size_t len, n; int subst; - char *p, *np, *new_prompt; + char *p, *np, *new_prompt, *endp; /* How much space do we need to malloc for the prompt? */ subst = 0; @@ -215,29 +215,35 @@ expand_prompt(old_prompt, user, host) } if (subst) { - new_prompt = (char *) emalloc(len + 1); + new_prompt = (char *) emalloc(++len); + *new_prompt = '\0'; + endp = new_prompt + len - 1; for (p = old_prompt, np = new_prompt; *p; p++) { if (p[0] =='%') { switch (p[1]) { case 'h': p++; - strcpy(np, user_shost); - np += strlen(user_shost); + if ((n = strlcat(new_prompt, user_shost, len)) >= len) + goto oflow; + np += n; continue; case 'H': p++; - strcpy(np, user_host); - np += strlen(user_host); + if ((n = strlcat(new_prompt, user_host, len)) >= len) + goto oflow; + np += n; continue; case 'u': p++; - strcpy(np, user_name); - np += strlen(user_name); + if ((n = strlcat(new_prompt, user_name, len)) >= len) + goto oflow; + np += n; continue; case 'U': p++; - strcpy(np, *user_runas); - np += strlen(*user_runas); + if ((n = strlcat(new_prompt, *user_runas, len)) >= len) + goto oflow; + np += n; continue; case '%': /* convert %% -> % */ @@ -248,6 +254,8 @@ expand_prompt(old_prompt, user, host) break; } } + if (np >= endp) + goto oflow; *np++ = *p; } *np = '\0'; @@ -255,6 +263,12 @@ expand_prompt(old_prompt, user, host) new_prompt = old_prompt; return(new_prompt); + +oflow: + /* We pre-allocate enough space, so this should never happen. */ + (void) fprintf(stderr, "%s: internal error, expand_prompt() overflow\n", + Argv[0]); + exit(1); } /* diff --git a/env.c b/env.c index 00ee4d5b6..def9ee6f4 100644 --- a/env.c +++ b/env.c @@ -213,16 +213,20 @@ format_env(var, val) char *var; char *val; { - char *estring, *p; - size_t varlen, vallen; - - varlen = strlen(var); - vallen = strlen(val); - p = estring = (char *) emalloc(varlen + vallen + 2); - strcpy(p, var); - p += varlen; - *p++ = '='; - strcpy(p, val); + char *estring; + size_t esize; + + esize = strlen(var) + 1 + strlen(val) + 1; + estring = (char *) emalloc(esize); + + /* We pre-allocate enough space, so this should never overflow. */ + if (strlcpy(estring, var, esize) >= esize || + strlcat(estring, "=", esize) >= esize || + strlcat(estring, val, esize) >= esize) { + (void) fprintf(stderr, "%s: internal error, format_env() overflow\n", + Argv[0]); + exit(1); + } return(estring); } diff --git a/find_path.c b/find_path.c index 8414e5704..5fbeb4e0a 100644 --- a/find_path.c +++ b/find_path.c @@ -82,6 +82,7 @@ find_path(infile, outfile, path) char *origpath; /* so we can free path later */ char *result = NULL; /* result of path/file lookup */ int checkdot = 0; /* check current dir? */ + int len; /* length parameter */ if (strlen(infile) >= MAXPATHLEN) { (void) fprintf(stderr, "%s: path too long: %s\n", Argv[0], infile); @@ -93,7 +94,7 @@ find_path(infile, outfile, path) * there is no need to look at $PATH. */ if (strchr(infile, '/')) { - (void) strcpy(command, infile); + strlcpy(command, infile, sizeof(command)); /* paranoia */ if (sudo_goodpath(command)) { *outfile = command; return(FOUND); @@ -128,11 +129,11 @@ find_path(infile, outfile, path) /* * Resolve the path and exit the loop if found. */ - if (strlen(path) + strlen(infile) + 1 >= MAXPATHLEN) { + len = snprintf(command, sizeof(command), "%s/%s", path, infile); + if (len <= 0 || len >= sizeof(command)) { (void) fprintf(stderr, "%s: path too long: %s\n", Argv[0], infile); exit(1); } - (void) sprintf(command, "%s/%s", path, infile); if ((result = sudo_goodpath(command))) break; diff --git a/parse.c b/parse.c index d0e9bc336..ec54635fe 100644 --- a/parse.c +++ b/parse.c @@ -343,10 +343,9 @@ command_matches(cmnd, cmnd_args, path, sudoers_args) while ((dent = readdir(dirp)) != NULL) { /* ignore paths > MAXPATHLEN (XXX - log) */ - if (plen + NAMLEN(dent) >= sizeof(buf)) + if (strlcpy(buf, path, sizeof(buf)) >= sizeof(buf) || + strlcat(buf, dent->d_name, sizeof(buf)) >= sizeof(buf)) continue; - strcpy(buf, path); - strcat(buf, dent->d_name); /* only stat if basenames are the same */ if (strcmp(cmnd_base, dent->d_name) != 0 || stat(buf, &pst) == -1) diff --git a/parse.yacc b/parse.yacc index 3f831b0a7..5ed436338 100644 --- a/parse.yacc +++ b/parse.yacc @@ -1109,6 +1109,7 @@ append(src, dstp, dst_len, dst_size, separator) /* Assumes dst will be NULL if not set. */ if (dst == NULL) { dst = (char *) emalloc(BUFSIZ); + *dst = '\0'; *dst_size = BUFSIZ; *dst_len = 0; *dstp = dst; @@ -1124,12 +1125,10 @@ append(src, dstp, dst_len, dst_size, separator) } /* Copy src -> dst adding a separator if appropriate and adjust len. */ - dst += *dst_len; - *dst_len += src_len; - *dst = '\0'; if (separator) - (void) strcat(dst, separator); - (void) strcat(dst, src); + (void) strlcat(dst, separator, *dst_size); + (void) strlcat(dst, src, *dst_size); + *dst_len += src_len; } /* diff --git a/sudo.c b/sudo.c index 5081845ed..ac8152659 100644 --- a/sudo.c +++ b/sudo.c @@ -480,7 +480,7 @@ init_vars(sudo_mode) /* Default value for cmnd and cwd, overridden later. */ if (user_cmnd == NULL) user_cmnd = NewArgv[0]; - (void) strcpy(user_cwd, "unknown"); + (void) strlcpy(user_cwd, "unknown", sizeof(user_cwd)); /* * We avoid gethostbyname() if possible since we don't want @@ -556,7 +556,7 @@ init_vars(sudo_mode) if (!getcwd(user_cwd, sizeof(user_cwd))) { (void) fprintf(stderr, "%s: Can't get working directory!\n", Argv[0]); - (void) strcpy(user_cwd, "unknown"); + (void) strlcpy(user_cwd, "unknown", sizeof(user_cwd)); } } else set_perms(PERM_ROOT); @@ -598,7 +598,7 @@ init_vars(sudo_mode) /* set user_args */ if (NewArgc > 1) { char *to, **from; - size_t size; + size_t size, n; /* If MODE_SHELL not set then NewArgv is contiguous so just count */ if (!(sudo_mode & MODE_SHELL)) { @@ -610,10 +610,15 @@ init_vars(sudo_mode) } /* alloc and copy. */ - to = user_args = (char *) emalloc(size); - for (from = NewArgv + 1; *from; from++) { - (void) strcpy(to, *from); - to += strlen(*from); + user_args = (char *) emalloc(size); + for (to = user_args, from = NewArgv + 1; *from; from++) { + n = strlcpy(to, *from, size - (to - user_args)); + if (n >= size) { + (void) fprintf(stderr, + "%s: internal error, init_vars() overflow\n", Argv[0]); + exit(1); + } + to += n; *to++ = ' '; } *--to = '\0'; diff --git a/testsudoers.c b/testsudoers.c index fa379456e..889f8a96f 100644 --- a/testsudoers.c +++ b/testsudoers.c @@ -377,17 +377,23 @@ main(argc, argv) /* Fill in cmnd_args from NewArgv. */ if (NewArgc > 1) { - size_t size; char *to, **from; + size_t size, n; - size = (size_t) NewArgv[NewArgc-1] + strlen(NewArgv[NewArgc-1]) - - (size_t) NewArgv[1] + 1; + size = (size_t) (NewArgv[NewArgc-1] - NewArgv[1]) + + strlen(NewArgv[NewArgc-1]) + 1; user_args = (char *) emalloc(size); - for (to = user_args, from = &NewArgv[1]; *from; from++) { + for (to = user_args, from = NewArgv + 1; *from; from++) { + n = strlcpy(to, *from, size - (to - user_args)); + if (n >= size) { + (void) fprintf(stderr, + "%s: internal error, init_vars() overflow\n", Argv[0]); + exit(1); + } + to += n; *to++ = ' '; - (void) strcpy(to, *from); - to += strlen(*from); } + *--to = '\0'; } /* Initialize default values. */ -- 2.40.0