From f148fc29d2737eef087a6ce62d6369b1260a5327 Mon Sep 17 00:00:00 2001 From: Patrick Winnertz Date: Tue, 3 Feb 2009 19:45:39 +0100 Subject: [PATCH 1/4] Fixing a theoretical buffer overflow which was reported by Roland Illig if the concat function was called with more than 32 parameters there will be a buffer overflow. This will add a small check to ensure that we concat only 32 parameters. Signed-off-by: Patrick Winnertz --- mhl/string.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mhl/string.h b/mhl/string.h index ee74e1d48..da8658406 100644 --- a/mhl/string.h +++ b/mhl/string.h @@ -71,7 +71,7 @@ static inline char* __mhl_str_concat_hlp(const char* base, ...) va_start(args,base); char* a; /* note: we use ((char*)(1)) as terminator - NULL is a valid argument ! */ - while ((a = va_arg(args, char*))!=(char*)1) + while ((a = va_arg(args, char*))!=(char*)1 && count <= 31 ) { if (a) { From 51388bd012248fa30d0d18209ad02d5a2ffb63a5 Mon Sep 17 00:00:00 2001 From: Patrick Winnertz Date: Tue, 3 Feb 2009 20:09:40 +0100 Subject: [PATCH 2/4] Call va_end after the iteration as we need to free the list again. Signed-off-by: Patrick Winnertz --- mhl/string.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mhl/string.h b/mhl/string.h index da8658406..fcbbd864f 100644 --- a/mhl/string.h +++ b/mhl/string.h @@ -71,7 +71,7 @@ static inline char* __mhl_str_concat_hlp(const char* base, ...) va_start(args,base); char* a; /* note: we use ((char*)(1)) as terminator - NULL is a valid argument ! */ - while ((a = va_arg(args, char*))!=(char*)1 && count <= 31 ) + while ((a = va_arg(args, char*))!=(char*)1 && count < __STR_CONCAT_MAX ) { if (a) { @@ -81,6 +81,7 @@ static inline char* __mhl_str_concat_hlp(const char* base, ...) count++; } } + va_end(args); if (!count) return mhl_str_dup(""); From 34fe2312b4aec7be01470bb7be2453a96b53413a Mon Sep 17 00:00:00 2001 From: Patrick Winnertz Date: Wed, 4 Feb 2009 11:51:30 +0100 Subject: [PATCH 3/4] Added enhancements from Sergei which he attached to #241. This will remove the static coded max arguments to concat. Signed-off-by: Patrick Winnertz --- mhl/string.h | 66 ++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/mhl/string.h b/mhl/string.h index fcbbd864f..47f097f1e 100644 --- a/mhl/string.h +++ b/mhl/string.h @@ -51,56 +51,52 @@ static inline void mhl_str_toupper(char* str) *str = toupper(*str); } -#define __STR_CONCAT_MAX 32 +/* note: we use ((char*)(1)) as terminator - NULL is a valid argument ! */ +static const char * mhl_s_c_sep__ = (const char *)1; /* _NEVER_ call this function directly ! */ -static inline char* __mhl_str_concat_hlp(const char* base, ...) +static inline char* mhl_str_concat_hlp__(const char* va_start_dummy, ...) { - static const char* arg_ptr[__STR_CONCAT_MAX]; - static size_t arg_sz[__STR_CONCAT_MAX]; - int count = 0; - size_t totalsize = 0; - - if (base) - { - arg_ptr[0] = base; - arg_sz[0] = totalsize = strlen(base); - count = 1; - } + char * result; + size_t result_len = 0; + char * p; + const char * chunk; va_list args; - va_start(args,base); - char* a; - /* note: we use ((char*)(1)) as terminator - NULL is a valid argument ! */ - while ((a = va_arg(args, char*))!=(char*)1 && count < __STR_CONCAT_MAX ) + va_start(args,va_start_dummy); + while ((chunk = va_arg(args, const char*)) != mhl_s_c_sep__) { - if (a) + if (chunk) { - arg_ptr[count] = a; - arg_sz[count] = strlen(a); - totalsize += arg_sz[count]; - count++; + result_len += strlen (chunk); } } va_end(args); - if (!count) + if (result_len == 0) return mhl_str_dup(""); - /* now as we know how much to copy, allocate the buffer */ - char* buffer = (char*)mhl_mem_alloc_u(totalsize+2); - char* current = buffer; - int x=0; - for (x=0; x Date: Wed, 4 Feb 2009 12:00:44 +0100 Subject: [PATCH 4/4] Resolve some issues in mhl Rollang Illig pointed us to: - isspace & toupper needs an unsigned char as arg - collision with compiler namespace __X - resolved some typos Signed-off-by: Patrick Winnertz --- mhl/env.h | 4 ++-- mhl/escape.h | 8 ++++---- mhl/memory.h | 6 +++--- mhl/strhash.h | 4 ++-- mhl/string.h | 22 +++++++++++++--------- mhl/types.h | 8 +++++--- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/mhl/env.h b/mhl/env.h index 8d5633797..6598e0cbf 100644 --- a/mhl/env.h +++ b/mhl/env.h @@ -1,5 +1,5 @@ -#ifndef __MHL_ENV_H -#define __MHL_ENV_H +#ifndef MHL_ENV_H +#define MHL_ENV_H #include diff --git a/mhl/escape.h b/mhl/escape.h index 2ec4e0d46..118e08269 100644 --- a/mhl/escape.h +++ b/mhl/escape.h @@ -1,5 +1,5 @@ -#ifndef __MHL_SHELL_ESCAPE_H -#define __MHL_SHELL_ESCAPE_H +#ifndef MHL_ESCAPE_H +#define MHL_ESCAPE_H /* Micro helper library: shell escaping functions */ @@ -64,7 +64,7 @@ static inline SHELL_ESCAPED_STR mhl_shell_escape_dup(const char* src) /** Unescape paths or other strings for e.g the internal cd shell-unescape within a given buffer (writing to it!) - /params const char * in + /params const char * src string for unescaping /returns return unescaped string @@ -113,7 +113,7 @@ static inline char* mhl_shell_unescape_buf(char* text) case '`': case '"': case ';': - case '\0': /* end of line! malformed escape string */ + case '\0': /* end of string! malformed escape string */ goto out; default: (*writeptr) = c; writeptr++; break; diff --git a/mhl/memory.h b/mhl/memory.h index b00617700..3268e93a1 100644 --- a/mhl/memory.h +++ b/mhl/memory.h @@ -1,5 +1,5 @@ -#ifndef __MHL_MEM -#define __MHL_MEM +#ifndef MHL_MEMORY_H +#define MHL_MEMORY_H #include #include @@ -17,7 +17,7 @@ static inline void mhl_mem_free(void* ptr) } /* free an ptr and NULL it */ -#define MHL_PTR_FREE(ptr) do { mhl_mem_free(ptr); (ptr) = NULL; } while (0); +#define MHL_PTR_FREE(ptr) do { mhl_mem_free(ptr); (ptr) = NULL; } while (0) /* allocate a chunk on stack - automatically free'd on function exit */ #define mhl_stack_alloc(sz) (alloca(sz)) diff --git a/mhl/strhash.h b/mhl/strhash.h index 1f195053c..8ff17cba6 100644 --- a/mhl/strhash.h +++ b/mhl/strhash.h @@ -1,5 +1,5 @@ -#ifndef __MHL_STRHASH_H -#define __MHL_STRHASH_H +#ifndef MHL_STRHASH_H +#define MHL_STRHASH_H #include #include diff --git a/mhl/string.h b/mhl/string.h index 47f097f1e..3b4c421f1 100644 --- a/mhl/string.h +++ b/mhl/string.h @@ -1,5 +1,5 @@ -#ifndef __MHL_STRING_H -#define __MHL_STRING_H +#ifndef MHL_STRING_H +#define MHL_STRING_H #include #include @@ -10,6 +10,9 @@ #define mhl_str_ndup(str,len) ((str ? strndup(str,len) : strdup(""))) #define mhl_str_len(str) ((str ? strlen(str) : 0)) +#define ISSPACE(c) isspace((unsigned char)(c)) +#define TOUPPER(c) toupper((unsigned char)(c)) + static inline char * mhl_str_dup_range(const char * s_start, const char * s_bound) { return mhl_str_ndup(s_start, s_bound - s_start); @@ -20,26 +23,26 @@ static inline char* mhl_str_trim(char* str) if (!str) return NULL; /* NULL string ?! bail out. */ /* find the first non-space */ - char* start; for (start=str; ((*str) && (!isspace(*str))); str++); + char* start; for (start=str; ((*str) && (!ISSPACE(*str))); str++); /* only spaces ? */ if (!(*str)) { *str = 0; return str; } - /* get the size (cannot be empty - catched above) */ + /* get the size (cannot be empty - caught above) */ size_t _sz = strlen(str); /* find the proper end */ char* end; - for (end=(str+_sz-1); ((end>str) && (isspace(*end))); end--); + for (end=(str+_sz-1); ((end>str) && (ISSPACE(*end))); end--); end[1] = 0; /* terminate, just to be sure */ /* if we have no leading spaces, just trucate */ if (start==str) { end++; *end = 0; return str; } - /* if it' only one char, dont need memmove for that */ + /* if it's only one char, dont need memmove for that */ if (start==end) { str[0]=*start; str[1]=0; return str; } - /* by here we have a (non-empty) region between start end end */ + /* by here we have a (non-empty) region between start and end */ memmove(str,start,(end-start+1)); return str; } @@ -48,7 +51,7 @@ static inline void mhl_str_toupper(char* str) { if (str) for (;*str;str++) - *str = toupper(*str); + *str = TOUPPER(*str); } /* note: we use ((char*)(1)) as terminator - NULL is a valid argument ! */ @@ -127,6 +130,7 @@ static inline char * mhl_strmove(char * dest, const char * src) { size_t n = strlen (src) + 1; /* + '\0' */ + /* strictly speaking, this invokes undefined behavior as soon as dest and src are pointers into different objects. */ assert (dest<=src); return memmove(dest, src, n); @@ -165,4 +169,4 @@ static inline char* mhl_str_dir_plus_file(const char* dirname, const char* filen return buffer; } -#endif /* __MHL_STRING_H */ +#endif /* MHL_STRING_H */ diff --git a/mhl/types.h b/mhl/types.h index 1f8400299..977c27a2c 100644 --- a/mhl/types.h +++ b/mhl/types.h @@ -4,13 +4,15 @@ */ -#ifndef __MHL_TYPES_H -#define __MHL_TYPES_H +#ifndef MHL_TYPES_H +#define MHL_TYPES_H -typedef enum +#if !defined(__bool_true_false_are_defined) && !defined(false) && !defined(true) && !defined(bool) +typedef enum { false = 0, true = 1 } bool; +#endif #endif