>From ffaeb3e4fb97906bc2dd92f632fa727b842f65b5 Mon Sep 17 00:00:00 2001 From: Christian Kellermann Date: Fri, 10 Jan 2014 15:43:07 +0100 Subject: [PATCH] Replace unsafe string functions with their safer counterparts This patch removes strcpy, strcat and sprintf usage out of core in favor for strlcpy, strlcat and snprintf. For systems that don't include strlcat and strlcpy in their string.h these two functions have been imported from OpenBSD. Systems that *do* ship these functions should declare HAVE_STRLCPY and HAVE_STRLCAT in their chicken-config.h. The chicken definitions for strcpy and strcat have been removed. This has been done for the Makefile.bsd as these do ship with these procedures. Signed-off-by: Christian Kellermann --- Makefile.bsd | 2 ++ chicken.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++------- library.scm | 2 +- posix-common.scm | 2 +- posixunix.scm | 9 ++--- runtime.c | 85 ++++++++++++++++++++++++---------------------- tcp.scm | 4 +-- 7 files changed, 145 insertions(+), 59 deletions(-) diff --git a/Makefile.bsd b/Makefile.bsd index 0dbc247..4dd003f 100644 --- a/Makefile.bsd +++ b/Makefile.bsd @@ -86,6 +86,8 @@ chicken-config.h: chicken-defaults.h echo "#define HAVE_STRERROR 1" >>$@ echo "#define HAVE_STRINGS_H 1" >>$@ echo "#define HAVE_STRING_H 1" >>$@ + echo "#define HAVE_STRLCAT 1" >>$@ + echo "#define HAVE_STRLCPY 1" >>$@ echo "#define HAVE_STRTOLL 1" >>$@ echo "#define HAVE_STRTOQ 1" >>$@ echo "#define HAVE_SYS_STAT_H 1" >>$@ diff --git a/chicken.h b/chicken.h index 51adeba..296b9d6 100644 --- a/chicken.h +++ b/chicken.h @@ -1,3 +1,4 @@ + /* chicken.h - General headerfile for compiler generated executables ; ; Copyright (c) 2008-2014, The Chicken Team @@ -920,12 +921,12 @@ DECL_C_PROC_p0 (128, 1,0,0,0,0,0,0,0) # define C_memcpy memcpy # define C_memcmp memcmp -# define C_strcpy strcpy +# define C_strlcpy strlcpy # define C_strncpy strncpy # define C_strcmp strcmp # define C_strncmp strncmp # define C_strlen strlen -# define C_strcat strcat +# define C_strlcat strlcat # define C_memset memset # define C_memmove memmove # define C_strncasecmp strncasecmp @@ -942,7 +943,6 @@ DECL_C_PROC_p0 (128, 1,0,0,0,0,0,0,0) # define C_fopen fopen # define C_fclose fclose # define C_strpbrk strpbrk -# define C_sprintf sprintf # define C_snprintf snprintf # define C_printf printf # define C_fprintf fprintf @@ -1306,7 +1306,7 @@ extern double trunc(double); #define C_rename_file(old, new) C_fix(rename(C_c_string(old), C_c_string(new))) #define C_delete_file(fname) C_fix(remove(C_c_string(fname))) #define C_poke_double(b, i, n) (((double *)C_data_pointer(b))[ C_unfix(i) ] = C_c_double(n), C_SCHEME_UNDEFINED) -#define C_poke_c_string(b, i, from) (C_strcpy((char *)C_block_item(b, C_unfix(i)), C_data_pointer(from)), C_SCHEME_UNDEFINED) +#define C_poke_c_string(b, i, from, s) (C_strlcpy((char *)C_block_item(b, C_unfix(i)), C_data_pointer(from), s), C_SCHEME_UNDEFINED) #define C_peek_fixnum(b, i) C_fix(C_block_item(b, C_unfix(i))) #define C_peek_byte(ptr, i) C_fix(((unsigned char *)C_u_i_car(ptr))[ C_unfix(i) ]) #define C_dupstr(s) C_strdup(C_data_pointer(s)) @@ -2911,9 +2911,9 @@ C_path_to_executable(C_char *fname) pid = C_getpid(); # ifdef __linux__ - C_sprintf(linkname, "/proc/%i/exe", pid); + C_snprintf(linkname, sizeof(linkname), "/proc/%i/exe", pid); # else - C_sprintf(linkname, "/proc/%i/path/a.out", pid); /* SunOS / Solaris */ + C_snprintf(linkname, sizeof(linkname), "/proc/%i/path/a.out", pid); /* SunOS / Solaris */ # endif ret = C_readlink(linkname, buffer, C_MAX_PATH - 1); @@ -2963,7 +2963,7 @@ C_path_to_executable(C_char *fname) /* absolute path */ if(*fname == '/') { fname[ i ] = '\0'; - C_strcpy(buffer, fname); + C_strlcpy(buffer, fname, C_MAX_PATH); return buffer; } else { @@ -2971,8 +2971,8 @@ C_path_to_executable(C_char *fname) if(C_getcwd(buffer, C_MAX_PATH - 1) == NULL) return NULL; - C_strcat(buffer, "/"); - C_strcat(buffer, fname); + C_strlcat(buffer, "/", C_MAX_PATH); + C_strlcat(buffer, fname, C_MAX_PATH); if(C_access(buffer, F_OK) == 0) { for(i = C_strlen(buffer); i >= 0 && buffer[ i ] != '/'; --i); @@ -2997,8 +2997,8 @@ C_path_to_executable(C_char *fname) case ':': C_strncpy(buffer, path + j, k - j); buffer[ k - j ] = '\0'; - C_strcat(buffer, "/"); - C_strcat(buffer, fname); + C_strlcat(buffer, "/", C_MAX_PATH); + C_strlcat(buffer, fname, C_MAX_PATH); if(C_access(buffer, F_OK) == 0) { dname = C_strdup(buffer); @@ -3035,7 +3035,7 @@ C_path_to_executable(C_char *fname) while (get_next_image_info(0, &cookie, &info) == B_OK) { if (info.type == B_APP_IMAGE) { - C_strcpy(buffer, info.name); + C_strlcpy(buffer, info.name, C_MAX_PATH); for(i = C_strlen(buffer); i >= 0 && buffer[ i ] != '/'; --i); @@ -3053,6 +3053,82 @@ C_path_to_executable(C_char *fname) #endif +/* + * The following strlcat and strlcpy functions have this license: + * + * Copyright (c) 1998 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + + +#ifndef HAVE_STRLCPY +/* + * Copy src to string dst of size siz. At most siz-1 characters + * will be copied. Always NUL terminates (unless siz == 0). + * Returns strlen(src); if retval >= siz, truncation occurred. + */ +C_inline size_t +strlcpy(char *dst, const char *src, size_t siz) +{ + char *d = dst; + const char *s = src; + size_t n = siz; + /* Copy as many bytes as will fit */ + if (n != 0) { + while (--n != 0) { + if ((*d++ = *s++) == '\0') + break; + } + } + /* Not enough room in dst, add NUL and traverse rest of src */ + if (n == 0) { + if (siz != 0) + *d = '\0'; /* NUL-terminate dst */ + while (*s++) + ; + } + return(s - src - 1); /* count does not include NUL */ +} +#endif + +#ifndef HAVE_STRLCAT +C_inline size_t +strlcat(char *dst, const char *src, size_t siz) +{ + char *d = dst; + const char *s = src; + size_t n = siz; + size_t dlen; + /* Find the end of dst and adjust bytes left but don't go past end */ + while (n-- != 0 && *d != '\0') + d++; + dlen = d - dst; + n = siz - dlen; + if (n == 0) + return(dlen + strlen(s)); + while (*s != '\0') { + if (n != 1) { + *d++ = *s; + n--; + } + s++; + } + *d = '\0'; + return(dlen + (s - src)); /* count does not include NUL */ +} +#endif + C_END_C_DECLS #endif /* ___CHICKEN */ diff --git a/library.scm b/library.scm index 107e884..2acd721 100644 --- a/library.scm +++ b/library.scm @@ -4376,7 +4376,7 @@ EOF str2 ) ) (define (##sys#poke-c-string b i s) - (##core#inline "C_poke_c_string" b i (##sys#make-c-string s)) ) + (##core#inline "C_poke_c_string" b i (##sys#make-c-string s) s) ) (define (##sys#poke-integer b i n) (##core#inline "C_poke_integer" b i n)) (define (##sys#poke-double b i n) (##core#inline "C_poke_double" b i n)) diff --git a/posix-common.scm b/posix-common.scm index d14b4c6..e9a6904 100644 --- a/posix-common.scm +++ b/posix-common.scm @@ -385,7 +385,7 @@ EOF (begin (##core#inline "C_closedir" handle) '() ) - (let* ([flen (##core#inline "C_foundfile" entry buffer)] + (let* ([flen (##core#inline "C_foundfile" entry buffer (string-length buffer))] [file (##sys#substring buffer 0 flen)] [char1 (string-ref file 0)] [char2 (and (fx> flen 1) (string-ref file 1))] ) diff --git a/posixunix.scm b/posixunix.scm index 6a42911..a816fbc 100644 --- a/posixunix.scm +++ b/posixunix.scm @@ -147,7 +147,7 @@ static C_TLS struct stat C_statbuf; #define C_opendir(x,h) C_set_block_item(h, 0, (C_word) opendir(C_c_string(x))) #define C_closedir(h) (closedir((DIR *)C_block_item(h, 0)), C_SCHEME_UNDEFINED) #define C_readdir(h,e) C_set_block_item(e, 0, (C_word) readdir((DIR *)C_block_item(h, 0))) -#define C_foundfile(e,b) (strcpy(C_c_string(b), ((struct dirent *) C_block_item(e, 0))->d_name), C_fix(strlen(((struct dirent *) C_block_item(e, 0))->d_name))) +#define C_foundfile(e,b,l) (C_strlcpy(C_c_string(b), ((struct dirent *) C_block_item(e, 0))->d_name, l), C_fix(strlen(((struct dirent *) C_block_item(e, 0))->d_name))) #define open_binary_input_pipe(a, n, name) C_mpointer(a, popen(C_c_string(name), "r")) #define open_text_input_pipe(a, n, name) open_binary_input_pipe(a, n, name) @@ -205,12 +205,13 @@ static C_word C_fcall C_setenv(C_word x, C_word y) { char *sx = C_data_pointer(x), *sy = C_data_pointer(y); int n1 = C_strlen(sx), n2 = C_strlen(sy); - char *buf = (char *)C_malloc(n1 + n2 + 2); + int buf_len = n1 + n2 + 2; + char *buf = (char *)C_malloc(buf_len); if(buf == NULL) return(C_fix(0)); else { - C_strcpy(buf, sx); + C_strlcpy(buf, buf_len, sx); buf[ n1 ] = '='; - C_strcpy(buf + n1 + 1, sy); + C_strlcat(buf + n1 + 1, buf_len, sy); return(C_fix(putenv(buf))); } } diff --git a/runtime.c b/runtime.c index a58eb80..b8737a9 100644 --- a/runtime.c +++ b/runtime.c @@ -594,14 +594,15 @@ void parse_argv(C_char *cmds) for(bptr0 = bptr = buffer; !isspace((int)(*ptr)) && *ptr != '\0'; *(bptr++) = *(ptr++)) ++n; - + *bptr = '\0'; - aptr = (C_char *)malloc(sizeof(C_char) * (n + 1)); - - if(aptr == NULL) + + aptr = (char*) malloc(sizeof(C_char) * (n + 1)); + if (!aptr) panic(C_text("cannot allocate argument buffer")); - C_strcpy(aptr, bptr0); + C_strlcpy(aptr, bptr0, sizeof(C_char) * (n + 1)); + C_main_argv[ C_main_argc++ ] = aptr; } } @@ -1508,7 +1509,7 @@ void usual_panic(C_char *msg) C_dbg_hook(C_SCHEME_UNDEFINED); if(C_gui_mode) { - C_sprintf(buffer, C_text("%s\n\n%s"), msg, dmp); + C_snprintf(buffer, sizeof(buffer), C_text("%s\n\n%s"), msg, dmp); #if defined(_WIN32) && !defined(__CYGWIN__) MessageBox(NULL, buffer, C_text("CHICKEN runtime"), MB_OK | MB_ICONERROR); ExitProcess(1); @@ -1525,7 +1526,7 @@ void horror(C_char *msg) C_dbg_hook(C_SCHEME_UNDEFINED); if(C_gui_mode) { - C_sprintf(buffer, C_text("%s"), msg); + C_snprintf(buffer, sizeof(buffer), C_text("%s"), msg); #if defined(_WIN32) && !defined(__CYGWIN__) MessageBox(NULL, buffer, C_text("CHICKEN runtime"), MB_OK | MB_ICONERROR); ExitProcess(1); @@ -2516,7 +2517,7 @@ C_regparm C_word C_fcall C_string2_safe(C_word **ptr, int max, C_char *str) len = C_strlen(str); if(len >= max) { - C_sprintf(buffer, C_text("foreign string result exceeded maximum of %d bytes"), max); + C_snprintf(buffer, sizeof(buffer), C_text("foreign string result exceeded maximum of %d bytes"), max); panic(buffer); } @@ -3834,9 +3835,10 @@ C_char *C_dump_trace(int start) { TRACE_INFO *ptr; C_char *result; - int i; + int i, result_len; - if((result = (char *)C_malloc(STRING_BUFFER_SIZE)) == NULL) + result_len = STRING_BUFFER_SIZE; + if((result = (char *)C_malloc(result_len)) == NULL) horror(C_text("out of memory - cannot allocate trace-dump buffer")); *result = '\0'; @@ -3844,7 +3846,7 @@ C_char *C_dump_trace(int start) if(trace_buffer_top > trace_buffer || trace_buffer_full) { if(trace_buffer_full) { i = C_trace_buffer_size; - C_strcat(result, C_text("...more...\n")); + C_strlcat(result, C_text("...more...\n"), result_len); } else i = trace_buffer_top - trace_buffer; @@ -3856,14 +3858,16 @@ C_char *C_dump_trace(int start) if(ptr >= trace_buffer_limit) ptr = trace_buffer; if(C_strlen(result) > STRING_BUFFER_SIZE - 32) { - if((result = C_realloc(result, C_strlen(result) * 2)) == NULL) + result_len = C_strlen(result) * 2; + result = C_realloc(result, result_len); + if(result == NULL) horror(C_text("out of memory - cannot reallocate trace-dump buffer")); } - C_strcat(result, ptr->raw); + C_strlcat(result, ptr->raw, result_len); - if(i > 0) C_strcat(result, "\n"); - else C_strcat(result, " \t<--\n"); + if(i > 0) C_strlcat(result, "\n", result_len); + else C_strlcat(result, " \t<--\n", result_len); } } @@ -3966,18 +3970,17 @@ C_word C_halt(C_word msg) if(C_gui_mode) { if(msg != C_SCHEME_FALSE) { int n = C_header_size(msg); - + if (n >= sizeof(buffer)) n = sizeof(buffer) - 1; - C_strncpy(buffer, (C_char *)C_data_pointer(msg), n); - buffer[ n ] = '\0'; + C_strlcpy(buffer, (C_char *)C_data_pointer(msg), n); /* XXX msg isn't checked for NUL bytes, but we can't barf here either! */ } - else C_strcpy(buffer, C_text("(aborted)")); + else C_strlcpy(buffer, C_text("(aborted)"), sizeof(buffer)); - C_strcat(buffer, C_text("\n\n")); + C_strlcat(buffer, C_text("\n\n"), sizeof(buffer)); - if(dmp != NULL) C_strcat(buffer, dmp); + if(dmp != NULL) C_strlcat(buffer, dmp, sizeof(buffer)); #if defined(_WIN32) && !defined(__CYGWIN__) MessageBox(NULL, buffer, C_text("CHICKEN runtime"), MB_OK | MB_ICONERROR); @@ -7745,13 +7748,13 @@ void C_ccall C_number_to_string(C_word c, C_word closure, C_word k, C_word num, switch(radix) { #ifdef C_SIXTY_FOUR - case 8: C_sprintf(p = buffer + 1, C_text("%llo"), (long long)num); break; - case 10: C_sprintf(p = buffer + 1, C_text("%lld"), (long long)num); break; - case 16: C_sprintf(p = buffer + 1, C_text("%llx"), (long long)num); break; + case 8: C_snprintf(p = buffer + 1, sizeof(buffer) -1 , C_text("%llo"), (long long)num); break; + case 10: C_snprintf(p = buffer + 1, sizeof(buffer) - 1, C_text("%lld"), (long long)num); break; + case 16: C_snprintf(p = buffer + 1, sizeof(buffer) - 1, C_text("%llx"), (long long)num); break; #else - case 8: C_sprintf(p = buffer + 1, C_text("%o"), num); break; - case 10: C_sprintf(p = buffer + 1, C_text("%d"), num); break; - case 16: C_sprintf(p = buffer + 1, C_text("%x"), num); break; + case 8: C_snprintf(p = buffer + 1, sizeof(buffer) - 1, C_text("%o"), num); break; + case 10: C_snprintf(p = buffer + 1, sizeof(buffer) - 1, C_text("%d"), num); break; + case 16: C_snprintf(p = buffer + 1, sizeof(buffer) - 1, C_text("%x"), num); break; #endif default: p = to_n_nary(num, radix); @@ -7772,11 +7775,11 @@ void C_ccall C_number_to_string(C_word c, C_word closure, C_word k, C_word num, switch(radix) { case 8: - C_sprintf(p = buffer, "%o", (unsigned int)f); + C_snprintf(p = buffer, sizeof(buffer), "%o", (unsigned int)f); goto fini; case 16: - C_sprintf(p = buffer, "%x", (unsigned int)f); + C_snprintf(p = buffer, sizeof(buffer), "%x", (unsigned int)f); goto fini; case 10: break; /* force output of decimal point to retain @@ -7790,11 +7793,13 @@ void C_ccall C_number_to_string(C_word c, C_word closure, C_word k, C_word num, } if(C_isnan(f)) { - C_strcpy(p = buffer, "+nan.0"); + C_strlcpy(buffer, C_text("+nan.0"), sizeof(buffer)); + p = buffer; goto fini; } else if(C_isinf(f)) { - C_sprintf(p = buffer, "%cinf.0", f > 0 ? '+' : '-'); + C_snprintf(buffer, sizeof(buffer), "%cinf.0", f > 0 ? '+' : '-'); + p = buffer; goto fini; } @@ -7807,7 +7812,7 @@ void C_ccall C_number_to_string(C_word c, C_word closure, C_word k, C_word num, C_memmove(buffer + 1, buffer, C_strlen(buffer) + 1); *buffer = '+'; } - else if(buffer[ 1 ] != 'i') C_strcat(buffer, C_text(".0")); /* negative infinity? */ + else if(buffer[ 1 ] != 'i') C_strlcat(buffer, C_text(".0"), sizeof(buffer)); /* negative infinity? */ } p = buffer; @@ -7834,9 +7839,9 @@ C_fixnum_to_string(C_word c, C_word self, C_word k, C_word num) /*XXX is this necessary? */ #ifdef C_SIXTY_FOUR - C_sprintf(buffer, C_text(LONG_FORMAT_STRING), C_unfix(num)); + C_snprintf(buffer, sizeof(buffer), C_text(LONG_FORMAT_STRING), C_unfix(num)); #else - C_sprintf(buffer, C_text("%d"), C_unfix(num)); + C_snprintf(buffer, sizeof(buffer), C_text("%d"), C_unfix(num)); #endif n = C_strlen(buffer); a = C_alloc(C_bytestowords(n) + 1); @@ -8389,16 +8394,18 @@ void dload_2(void *dummy) C_char *topname = (C_char *)C_data_pointer(entry); C_char *mname = (C_char *)C_data_pointer(name); C_char *tmp; + int tmp_len = 0; if((handle = C_dlopen(mname, dlopen_flags)) != NULL) { if((p = C_dlsym(handle, topname)) == NULL) { - tmp = (C_char *)C_malloc(C_strlen(topname) + 2); - + tmp_len = C_strlen(topname) + 2; + tmp = (C_char *)C_malloc(tmp_len); + if(tmp == NULL) panic(C_text("out of memory - cannot allocate toplevel name string")); - - C_strcpy(tmp, C_text("_")); - C_strcat(tmp, topname); + + C_strlcpy(tmp, C_text("_"), tmp_len); + C_strlcat(tmp, topname, tmp_len); p = C_dlsym(handle, tmp); C_free(tmp); } diff --git a/tcp.scm b/tcp.scm index df31cda..400e944 100644 --- a/tcp.scm +++ b/tcp.scm @@ -165,7 +165,7 @@ EOF "int len = sizeof(struct sockaddr_in);" "if(getsockname(s, (struct sockaddr *)&sa, (socklen_t *)&len) != 0) C_return(NULL);" "ptr = (unsigned char *)&sa.sin_addr;" - "sprintf(addr_buffer, \"%d.%d.%d.%d\", ptr[ 0 ], ptr[ 1 ], ptr[ 2 ], ptr[ 3 ]);" + "snprintf(addr_buffer, sizeof(addr_buffer), \"%d.%d.%d.%d\", ptr[ 0 ], ptr[ 1 ], ptr[ 2 ], ptr[ 3 ]);" "C_return(addr_buffer);") ) (define ##net#getsockport @@ -189,7 +189,7 @@ EOF "unsigned int len = sizeof(struct sockaddr_in);" "if(getpeername(s, (struct sockaddr *)&sa, ((socklen_t *)&len)) != 0) C_return(NULL);" "ptr = (unsigned char *)&sa.sin_addr;" - "sprintf(addr_buffer, \"%d.%d.%d.%d\", ptr[ 0 ], ptr[ 1 ], ptr[ 2 ], ptr[ 3 ]);" + "snprintf(addr_buffer, sizeof(addr_buffer), \"%d.%d.%d.%d\", ptr[ 0 ], ptr[ 1 ], ptr[ 2 ], ptr[ 3 ]);" "C_return(addr_buffer);") ) (define ##net#startup -- 1.8.5.2