From c6ec6410afdb21cc6f2ecdf0d36559dc8f0fc6cd Mon Sep 17 00:00:00 2001 From: Benjamin Kaduk Date: Mon, 9 Feb 2015 10:38:04 -0500 Subject: [PATCH] Avoid unsafe scanf("%s") Reading user input into a fixed-length buffer just to check the first character is silly and an easy buffer overrun. gcc on Ubuntu 13.03 warns about the unchecked return value for scanf(), but scanf("%s") is guaranteed to either succeed or get EOF/EINTR/etc.. In any case, we don't need to use scanf() at all, here -- reuse an idiom from BSD cp(1) and loop around getchar to read the user's response, eliminating the fixed-length buffer entirely. A separate initial loop is needed to skip leading whitespace, which is done implicitly by scanf(). Change-Id: Ic5ed65e80146aa3d08a4b03c213f748ef088156b Reviewed-on: http://gerrit.openafs.org/11758 Tested-by: BuildBot Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk Reviewed-by: Perry Ruiter Reviewed-by: Michael Meffie Reviewed-by: Jeffrey Altman --- src/uss/uss_vol.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/uss/uss_vol.c b/src/uss/uss_vol.c index 5ea368941e..c9c13941da 100644 --- a/src/uss/uss_vol.c +++ b/src/uss/uss_vol.c @@ -34,6 +34,8 @@ #include #include +#include + #include "uss_vol.h" /*Interface to this module */ #include "uss_common.h" /*Common definitions */ #include "uss_procs.h" /*Defs from procs module */ @@ -584,7 +586,7 @@ uss_vol_CreateVol(char *a_volname, char *a_server, char *a_partition, char *Oldmpoint = NULL; /*Old mountpoint name, if any */ char tmp_str[uss_MAX_SIZE]; /*Useful string buffer */ int o; /*Owner's user id */ - char userinput[64]; /*User's input */ + int checkch, ch; /*Read user's confirmation input */ struct uss_subdir *new_dir; /*Used to remember original ACL */ /* @@ -698,8 +700,12 @@ uss_vol_CreateVol(char *a_volname, char *a_server, char *a_partition, printf ("Overwrite files in pre-existing '%s' volume? [y, n]: ", a_volname); - scanf("%s", userinput); - if ((userinput[0] == 'y') || (userinput[0] == 'Y')) { + checkch = ch = ' '; + while (isspace(ch)) + checkch = ch = getchar(); + while (ch != '\n' && ch != EOF) + ch = getchar(); + if (checkch == 'y' || checkch == 'Y') { printf("\t[Overwriting allowed]\n"); uss_OverwriteThisOne = 1; } else