From d17a07767d703287595f3c96e72e0ecc8455bbf0 Mon Sep 17 00:00:00 2001 From: Cheyenne Wills Date: Thu, 12 Sep 2024 10:05:55 -0600 Subject: [PATCH] uss: Replace strcat with safer method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grammar.y file uses a series of strcat's to build the accesslist from the parsed tokens. There is no checking to see if the result exceeds the size of the output buffer. Replace the strcpy/strcat's with a simple snprintf that concatenates the tokens, and check to see if the snprintf failed. If there was an error concatenating the tokens, emit a message. NOTE: With --enable-checking a build error occurs on an Ubuntu 24.04 system, where the default _FORTIFY_SOURCE is set to 3 (hardened). The build produces the following: ... inlined from ‘yyparse’ at ./grammar.y:130:26: /usr/include/.../string_fortified.h:130:10: error: ‘__builtin___strcat_chk’ writing 2 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=] 130 | return __builtin___strcat_chk (__dest, __src, __glibc_objsize (__dest)); ...(repeated for the other uses of strcat)... The build error can be duplicated by setting _FORTIFY_SOURCE to 3. Reviewed-on: https://gerrit.openafs.org/15845 Tested-by: BuildBot Reviewed-by: Cheyenne Wills Reviewed-by: Michael Meffie Reviewed-by: Andrew Deason (cherry picked from commit 00b31c7bae017cbda9d9cf9b7d61299f882d9f12) Change-Id: If5dcf75098443e03e9c843039f22e8b414c34d66 Reviewed-on: https://gerrit.openafs.org/15857 Reviewed-by: Andrew Deason Tested-by: BuildBot Reviewed-by: Mark Vitale Reviewed-by: Michael Meffie Reviewed-by: Cheyenne Wills Tested-by: Cheyenne Wills Reviewed-by: Benjamin Kaduk --- src/uss/grammar.y | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/uss/grammar.y b/src/uss/grammar.y index 77a927ec8e..186c6c6cc3 100644 --- a/src/uss/grammar.y +++ b/src/uss/grammar.y @@ -123,12 +123,20 @@ entry : DIR_TKN accesslist : /* empty */ - {strcpy($$," ");} + { + if (strlcpy($$, " ", sizeof($$)) >= sizeof($$)) { + uss_procs_PrintErr(line-1, "Internal error, incorrect size for accesslist buffer\n"); + exit(1); + } + } | STRING_TKN STRING_TKN accesslist - {strcat($1," "); strcat($2," ");strcat($1,strcat($2,$3));strcpy($$,$1);} - + { + if (snprintf($$, sizeof($$), "%s %s %s", $1, $2, $3) >= sizeof($$)) { + uss_procs_PrintErr(line-1, " error in access list near \"%s\"\n", yylval.strval); + } + } ; %%