From ba009e5fd34e43bbf48f0fb05c1a5ee9ef2664ed Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 5 Jan 2015 06:05:56 +0200 Subject: [PATCH] Fixes to Coverity defects 85010 84878 72752 72742 72719 and 73418. skygw_utils.cc: Added function is_valid_posix_path that checks if a path is POSIX-compliant. --- query_classifier/query_classifier.cc | 2 +- server/core/modutil.c | 5 ++- server/core/test/testadminusers.c | 2 ++ server/modules/filter/hint/hintparser.c | 2 +- server/modules/protocol/mysql_backend.c | 2 +- .../routing/readwritesplit/readwritesplit.c | 2 +- utils/skygw_types.h | 1 + utils/skygw_utils.cc | 34 ++++++++++++++----- utils/skygw_utils.h | 2 +- 9 files changed, 36 insertions(+), 16 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 788f6e5c6..19345ccd6 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -175,7 +175,7 @@ bool parse_query ( len = MYSQL_GET_PACKET_LEN(data)-1; /*< distract 1 for packet type byte */ - if (len < 1 || (query_str = (char *)malloc(len+1)) == NULL) + if (len < 1 || len >= SIZE_MAX - 1 || (query_str = (char *)malloc(len+1)) == NULL) { /** Free parsing info data */ parsing_info_done(pi); diff --git a/server/core/modutil.c b/server/core/modutil.c index 2e6651595..667b186f2 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -140,7 +140,6 @@ int modutil_MySQL_query_len( { int len; int buflen; - uint8_t data; if (!modutil_is_SQL(buf)) { @@ -287,7 +286,7 @@ modutil_get_query(GWBUF *buf) case MYSQL_COM_QUERY: len = MYSQL_GET_PACKET_LEN(packet)-1; /*< distract 1 for packet type byte */ - if (len < 1 || (query_str = (char *)malloc(len+1)) == NULL) + if (len < 1 || len > SIZE_MAX - 1 || (query_str = (char *)malloc(len+1)) == NULL) { goto retblock; } @@ -297,7 +296,7 @@ modutil_get_query(GWBUF *buf) default: len = strlen(STRPACKETTYPE(packet_type))+1; - if (len < 1 || (query_str = (char *)malloc(len+1)) == NULL) + if (len < 1 || len > SIZE_MAX - 1 || (query_str = (char *)malloc(len+1)) == NULL) { goto retblock; } diff --git a/server/core/test/testadminusers.c b/server/core/test/testadminusers.c index ec2917783..866f7fff3 100644 --- a/server/core/test/testadminusers.c +++ b/server/core/test/testadminusers.c @@ -272,6 +272,8 @@ char *home, buf[1024]; if ((home = getenv("MAXSCALE_HOME")) == NULL || strlen(home) >= 1024) home = "/usr/local/skysql"; sprintf(buf, "%s/etc/passwd", home); + if(!is_valid_posix_path(buf)) + exit(1); if (strcmp(buf, "/etc/passwd") != 0) unlink(buf); diff --git a/server/modules/filter/hint/hintparser.c b/server/modules/filter/hint/hintparser.c index 91140ed97..abf2fb141 100644 --- a/server/modules/filter/hint/hintparser.c +++ b/server/modules/filter/hint/hintparser.c @@ -213,7 +213,7 @@ HINT_MODE mode = HM_EXECUTE; /* * If we have got here then we have a comment, ptr point to * the comment character if it is a '#' comment or the second - * character of the comment if it is a -- or /* comment + * character of the comment if it is a -- or \/\* comment * * Move to the next character in the SQL. */ diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 32c0fb277..1056f9026 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -1602,7 +1602,7 @@ static GWBUF* process_response_data ( if (nbytes_left == 0) { /** No more packets in this response */ - if (npackets_left == 0) + if (npackets_left == 0 && outbuf != NULL) { GWBUF* b = outbuf; diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 751d251d1..74bde276f 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3844,7 +3844,7 @@ static bool execute_sescmd_in_backend( tmpbuf = scur->scmd_cur_cmd->my_sescmd_buf; qlen = MYSQL_GET_PACKET_LEN((unsigned char*)tmpbuf->start); memset(data->db,0,MYSQL_DATABASE_MAXLEN+1); - if(qlen > 0) + if(qlen > 0 && qlen < UINT_MAX) strncpy(data->db,tmpbuf->start+5,qlen - 1); } /** Fallthrough */ diff --git a/utils/skygw_types.h b/utils/skygw_types.h index 4410cc23a..c3bd8b491 100644 --- a/utils/skygw_types.h +++ b/utils/skygw_types.h @@ -20,6 +20,7 @@ #include #include +#include #define SECOND_USEC (1024*1024L) #define MSEC_USEC (1024L) diff --git a/utils/skygw_utils.cc b/utils/skygw_utils.cc index 3e8e15e67..272a54f9a 100644 --- a/utils/skygw_utils.cc +++ b/utils/skygw_utils.cc @@ -2058,11 +2058,29 @@ size_t get_decimal_len( return value > 0 ? (size_t) log10 ((double) value) + 1 : 1; } - - - - - - - - +/** + * Check if the provided pathname is POSIX-compliant. The valid characters + * are [a-z A-Z 0-9._-]. + * @param path A null-terminated string + * @return true if it is a POSIX-compliant pathname, otherwise false + */ +bool is_valid_posix_path(char* path) +{ + char* ptr = path; + while (*ptr != '\0') + { + if (isalnum (*ptr) || + *ptr == '/' || + *ptr == '.' || + *ptr == '-' || + *ptr == '_') + { + ptr++; + } + else + { + return false; + } + } + return true; +} \ No newline at end of file diff --git a/utils/skygw_utils.h b/utils/skygw_utils.h index 2379dbb03..eb1fb0371 100644 --- a/utils/skygw_utils.h +++ b/utils/skygw_utils.h @@ -198,7 +198,7 @@ size_t get_decimal_len(size_t s); char* replace_literal(char* haystack, const char* needle, const char* replacement); - +bool is_valid_posix_path(char* path); EXTERN_C_BLOCK_END #endif /* SKYGW_UTILS_H */