From e17466980806a73306ae16204f1aea5121db6e62 Mon Sep 17 00:00:00 2001 From: xywang Date: Wed, 14 Jul 2021 13:54:45 +0800 Subject: [PATCH 1/4] [TD-5169]: fixed a potential crash bug --- src/plugins/http/inc/httpSql.h | 3 ++ src/plugins/http/src/httpGcHandle.c | 37 +++++++++++++++-- src/plugins/http/src/httpSql.c | 62 +++++++++++++++++++++++++++++ src/plugins/http/src/httpTgHandle.c | 17 +++++++- 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/plugins/http/inc/httpSql.h b/src/plugins/http/inc/httpSql.h index db3e3a3b16..325545af47 100644 --- a/src/plugins/http/inc/httpSql.h +++ b/src/plugins/http/inc/httpSql.h @@ -35,4 +35,7 @@ void httpTrimTableName(char *name); int32_t httpShrinkTableName(HttpContext *pContext, int32_t pos, char *name); char * httpGetCmdsString(HttpContext *pContext, int32_t pos); +int32_t httpCheckAllocEscapeSql(char *oldSql, char **newSql); +void httpCheckFreeEscapedSql(char *oldSql, char *newSql); + #endif diff --git a/src/plugins/http/src/httpGcHandle.c b/src/plugins/http/src/httpGcHandle.c index 925c74e7cd..ed3a28567e 100644 --- a/src/plugins/http/src/httpGcHandle.c +++ b/src/plugins/http/src/httpGcHandle.c @@ -176,6 +176,20 @@ bool gcProcessQueryRequest(HttpContext* pContext) { return false; } +#define ESCAPE_ERROR_PROC(code, context, root) \ + do { \ + if (code != 0) { \ + if (code == 1) { \ + httpSendErrorResp(context, TSDB_CODE_HTTP_GC_REQ_PARSE_ERROR); \ + } else { \ + httpSendErrorResp(context, TSDB_CODE_HTTP_NO_ENOUGH_MEMORY); \ + } \ + \ + cJSON_Delete(root); \ + return false; \ + } \ + } while (0) + for (int32_t i = 0; i < size; ++i) { cJSON* query = cJSON_GetArrayItem(root, i); if (query == NULL) continue; @@ -186,7 +200,14 @@ bool gcProcessQueryRequest(HttpContext* pContext) { continue; } - int32_t refIdBuffer = httpAddToSqlCmdBuffer(pContext, refId->valuestring); + char *newStr = NULL; + int32_t retCode = 0; + + retCode = httpCheckAllocEscapeSql(refId->valuestring, &newStr); + ESCAPE_ERROR_PROC(retCode, pContext, root); + + int32_t refIdBuffer = httpAddToSqlCmdBuffer(pContext, newStr); + httpCheckFreeEscapedSql(refId->valuestring, newStr); if (refIdBuffer == -1) { httpWarn("context:%p, fd:%d, user:%s, refId buffer is full", pContext, pContext->fd, pContext->user); break; @@ -195,7 +216,11 @@ bool gcProcessQueryRequest(HttpContext* pContext) { cJSON* alias = cJSON_GetObjectItem(query, "alias"); int32_t aliasBuffer = -1; if (!(alias == NULL || alias->valuestring == NULL || strlen(alias->valuestring) == 0)) { - aliasBuffer = httpAddToSqlCmdBuffer(pContext, alias->valuestring); + retCode = httpCheckAllocEscapeSql(alias->valuestring, &newStr); + ESCAPE_ERROR_PROC(retCode, pContext, root); + + aliasBuffer = httpAddToSqlCmdBuffer(pContext, newStr); + httpCheckFreeEscapedSql(alias->valuestring, newStr); if (aliasBuffer == -1) { httpWarn("context:%p, fd:%d, user:%s, alias buffer is full", pContext, pContext->fd, pContext->user); break; @@ -211,7 +236,11 @@ bool gcProcessQueryRequest(HttpContext* pContext) { continue; } - int32_t sqlBuffer = httpAddToSqlCmdBuffer(pContext, sql->valuestring); + retCode = httpCheckAllocEscapeSql(sql->valuestring, &newStr); + ESCAPE_ERROR_PROC(retCode, pContext, root); + + int32_t sqlBuffer = httpAddToSqlCmdBuffer(pContext, newStr); + httpCheckFreeEscapedSql(sql->valuestring, newStr); if (sqlBuffer == -1) { httpWarn("context:%p, fd:%d, user:%s, sql buffer is full", pContext, pContext->fd, pContext->user); break; @@ -237,6 +266,8 @@ bool gcProcessQueryRequest(HttpContext* pContext) { } } +#undef ESCAPE_ERROR_PROC + pContext->reqType = HTTP_REQTYPE_MULTI_SQL; pContext->encodeMethod = &gcQueryMethod; pContext->multiCmds->pos = 0; diff --git a/src/plugins/http/src/httpSql.c b/src/plugins/http/src/httpSql.c index 5a0480b694..20d1e159b3 100644 --- a/src/plugins/http/src/httpSql.c +++ b/src/plugins/http/src/httpSql.c @@ -423,3 +423,65 @@ void httpProcessRequest(HttpContext *pContext) { httpExecCmd(pContext); } } + +int32_t httpCheckAllocEscapeSql(char *oldSql, char **newSql) +{ + char *pos; + + if (oldSql == NULL || newSql == NULL) { + return 0; + } + + /* bad sql clause */ + pos = strstr(oldSql, "%%"); + if (pos) { + httpError("bad sql:%s", oldSql); + return 1; + } + + pos = strchr(oldSql, '%'); + if (pos == NULL) { + httpDebug("sql:%s", oldSql); + *newSql = oldSql; + return 0; + } + + *newSql = (char *) calloc(1, (strlen(oldSql) << 1) + 1); + if (newSql == NULL) { + httpError("failed to allocate for new sql, old sql:%s", oldSql); + return -1; + } + + char *src = oldSql; + char *dst = *newSql; + size_t sqlLen = strlen(src); + + while (1) { + memcpy(dst, src, pos - src + 1); + dst += pos - src + 1; + *dst++ = '%'; + + if (pos + 1 >= oldSql + sqlLen) { + break; + } + + src = ++pos; + pos = strchr(pos, '%'); + if (pos == NULL) { + memcpy(dst, src, sqlLen - strlen(src)); + break; + } + } + + return 0; +} + +void httpCheckFreeEscapedSql(char *oldSql, char *newSql) +{ + if (oldSql && newSql) { + if (oldSql != newSql) { + free(newSql); + } + } +} + diff --git a/src/plugins/http/src/httpTgHandle.c b/src/plugins/http/src/httpTgHandle.c index 69ac3e19c5..8aa156b84a 100644 --- a/src/plugins/http/src/httpTgHandle.c +++ b/src/plugins/http/src/httpTgHandle.c @@ -610,7 +610,22 @@ bool tgProcessSingleMetric(HttpContext *pContext, cJSON *metric, char *db) { // stable tag for detail for (int32_t i = 0; i < orderTagsLen; ++i) { cJSON *tag = orderedTags[i]; - stable_cmd->tagNames[i] = table_cmd->tagNames[i] = httpAddToSqlCmdBuffer(pContext, tag->string); + + char *tagStr = NULL; + int32_t retCode = httpCheckAllocEscapeSql(tag->string, &tagStr); + if (retCode != 0) { + if (retCode == 1) { + httpSendErrorResp(pContext, TSDB_CODE_HTTP_TG_INVALID_JSON); + } else { + httpSendErrorResp(pContext, TSDB_CODE_HTTP_NO_ENOUGH_MEMORY); + } + + return false; + } + + stable_cmd->tagNames[i] = table_cmd->tagNames[i] = httpAddToSqlCmdBuffer(pContext, tagStr); + + httpCheckFreeEscapedSql(tag->string, tagStr); if (tag->type == cJSON_String) stable_cmd->tagValues[i] = table_cmd->tagValues[i] = httpAddToSqlCmdBuffer(pContext, "'%s'", tag->valuestring); From cf76ed44fe396003a970f79c6c9ca882a85efe0f Mon Sep 17 00:00:00 2001 From: xywang Date: Wed, 14 Jul 2021 16:45:32 +0800 Subject: [PATCH 2/4] [TD-5169]: fixed a parsing bug --- src/plugins/http/src/httpSql.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/http/src/httpSql.c b/src/plugins/http/src/httpSql.c index 20d1e159b3..b2480dcad8 100644 --- a/src/plugins/http/src/httpSql.c +++ b/src/plugins/http/src/httpSql.c @@ -468,7 +468,7 @@ int32_t httpCheckAllocEscapeSql(char *oldSql, char **newSql) src = ++pos; pos = strchr(pos, '%'); if (pos == NULL) { - memcpy(dst, src, sqlLen - strlen(src)); + memcpy(dst, src, strlen(src)); break; } } From cc13cd44c37d65e32f372be04299164ecd4278f4 Mon Sep 17 00:00:00 2001 From: liuyq-617 Date: Wed, 14 Jul 2021 17:25:49 +0800 Subject: [PATCH 3/4] [TD-5260]add case for a potential crash --- tests/script/general/http/grafana_bug.sim | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/script/general/http/grafana_bug.sim b/tests/script/general/http/grafana_bug.sim index 0816e88f3f..ed184e17c6 100644 --- a/tests/script/general/http/grafana_bug.sim +++ b/tests/script/general/http/grafana_bug.sim @@ -247,4 +247,25 @@ if $system_content != @[{"refId":"A","target":"{val1:nil, val2:nil}","datapoints return -1 endi +sql create table tt (ts timestamp ,i int) tags(j binary(20),k binary(20)); +sql insert into t1 using tt tags('jnetworki','t1') values('2020-01-01 00:00:00.000',1)('2020-01-01 00:01:00.000',2)('2020-01-01 00:02:00.000',3)('2020-01-01 00:03:00.000',4)('2020-01-01 00:04:00.000',5); + +system_content curl -H 'Authorization: Basic cm9vdDp0YW9zZGF0YQ==' -d '[ {"refId":"A","alias":"","sql":"select max(i) from db.tt where j like \u0027%network%\u0027 and ts >= \u00272020-01-01 00:00:00.000\u0027 and ts < \u00272020-01-01 00:05:00.000\u0027 interval(5m) group by k "} ]' 127.0.0.1:7111/grafana/query +print step1-> $system_content +if $system_content != @[{"refId":"A","target":"{k:t1}","datapoints":[[5,1577808000000]]}]@ then + return -1 +endi + +system_content curl -H 'Authorization: Basic cm9vdDp0YW9zZGF0YQ==' -d '[ {"refId":"A","alias":"","sql":"select max(i) from db.tt where j like \u0027jnetwo%\u0027 and ts >= \u00272020-01-01 00:00:00.000\u0027 and ts < \u00272020-01-01 00:05:00.000\u0027 interval(5m) group by k "} ]' 127.0.0.1:7111/grafana/query +print step1-> $system_content +if $system_content != @[{"refId":"A","target":"{k:t1}","datapoints":[[5,1577808000000]]}]@ then + return -1 +endi + +system_content curl -H 'Authorization: Basic cm9vdDp0YW9zZGF0YQ==' -d '[ {"refId":"A","alias":"","sql":"select max(i) from db.tt where j like \u0027%networki\u0027 and ts >= \u00272020-01-01 00:00:00.000\u0027 and ts < \u00272020-01-01 00:05:00.000\u0027 interval(5m) group by k "} ]' 127.0.0.1:7111/grafana/query +print step1-> $system_content +if $system_content != @[{"refId":"A","target":"{k:t1}","datapoints":[[5,1577808000000]]}]@ then + return -1 +endi + system sh/exec.sh -n dnode1 -s stop -x SIGINT \ No newline at end of file From d2941c06088c20e150f9faf459cf000e054ebf8e Mon Sep 17 00:00:00 2001 From: xywang Date: Thu, 15 Jul 2021 13:17:10 +0800 Subject: [PATCH 4/4] [TD-5169]: simplified function implementation --- src/inc/taoserror.h | 2 ++ src/plugins/http/src/httpGcHandle.c | 8 ++------ src/plugins/http/src/httpSql.c | 10 +++++----- src/plugins/http/src/httpTgHandle.c | 8 ++------ src/util/src/terror.c | 2 ++ 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/inc/taoserror.h b/src/inc/taoserror.h index 1e996be889..b18aa2c2d9 100644 --- a/src/inc/taoserror.h +++ b/src/inc/taoserror.h @@ -395,6 +395,8 @@ int32_t* taosGetErrno(); #define TSDB_CODE_HTTP_OP_VALUE_NULL TAOS_DEF_ERROR_CODE(0, 0x11A5) //"value not find") #define TSDB_CODE_HTTP_OP_VALUE_TYPE TAOS_DEF_ERROR_CODE(0, 0x11A6) //"value type should be boolean number or string") +#define TSDB_CODE_HTTP_REQUEST_JSON_ERROR TAOS_DEF_ERROR_CODE(0, 0x1F00) //"http request json error") + // odbc #define TSDB_CODE_ODBC_OOM TAOS_DEF_ERROR_CODE(0, 0x2100) //"out of memory") #define TSDB_CODE_ODBC_CONV_CHAR_NOT_NUM TAOS_DEF_ERROR_CODE(0, 0x2101) //"convertion not a valid literal input") diff --git a/src/plugins/http/src/httpGcHandle.c b/src/plugins/http/src/httpGcHandle.c index ed3a28567e..883afcc4ec 100644 --- a/src/plugins/http/src/httpGcHandle.c +++ b/src/plugins/http/src/httpGcHandle.c @@ -178,12 +178,8 @@ bool gcProcessQueryRequest(HttpContext* pContext) { #define ESCAPE_ERROR_PROC(code, context, root) \ do { \ - if (code != 0) { \ - if (code == 1) { \ - httpSendErrorResp(context, TSDB_CODE_HTTP_GC_REQ_PARSE_ERROR); \ - } else { \ - httpSendErrorResp(context, TSDB_CODE_HTTP_NO_ENOUGH_MEMORY); \ - } \ + if (code != TSDB_CODE_SUCCESS) { \ + httpSendErrorResp(context, code); \ \ cJSON_Delete(root); \ return false; \ diff --git a/src/plugins/http/src/httpSql.c b/src/plugins/http/src/httpSql.c index b2480dcad8..c2e723732a 100644 --- a/src/plugins/http/src/httpSql.c +++ b/src/plugins/http/src/httpSql.c @@ -429,27 +429,27 @@ int32_t httpCheckAllocEscapeSql(char *oldSql, char **newSql) char *pos; if (oldSql == NULL || newSql == NULL) { - return 0; + return TSDB_CODE_SUCCESS; } /* bad sql clause */ pos = strstr(oldSql, "%%"); if (pos) { httpError("bad sql:%s", oldSql); - return 1; + return TSDB_CODE_HTTP_REQUEST_JSON_ERROR; } pos = strchr(oldSql, '%'); if (pos == NULL) { httpDebug("sql:%s", oldSql); *newSql = oldSql; - return 0; + return TSDB_CODE_SUCCESS; } *newSql = (char *) calloc(1, (strlen(oldSql) << 1) + 1); if (newSql == NULL) { httpError("failed to allocate for new sql, old sql:%s", oldSql); - return -1; + return TSDB_CODE_HTTP_NO_ENOUGH_MEMORY; } char *src = oldSql; @@ -473,7 +473,7 @@ int32_t httpCheckAllocEscapeSql(char *oldSql, char **newSql) } } - return 0; + return TSDB_CODE_SUCCESS; } void httpCheckFreeEscapedSql(char *oldSql, char *newSql) diff --git a/src/plugins/http/src/httpTgHandle.c b/src/plugins/http/src/httpTgHandle.c index 8aa156b84a..32516b9fd1 100644 --- a/src/plugins/http/src/httpTgHandle.c +++ b/src/plugins/http/src/httpTgHandle.c @@ -613,12 +613,8 @@ bool tgProcessSingleMetric(HttpContext *pContext, cJSON *metric, char *db) { char *tagStr = NULL; int32_t retCode = httpCheckAllocEscapeSql(tag->string, &tagStr); - if (retCode != 0) { - if (retCode == 1) { - httpSendErrorResp(pContext, TSDB_CODE_HTTP_TG_INVALID_JSON); - } else { - httpSendErrorResp(pContext, TSDB_CODE_HTTP_NO_ENOUGH_MEMORY); - } + if (retCode != TSDB_CODE_SUCCESS) { + httpSendErrorResp(pContext, retCode); return false; } diff --git a/src/util/src/terror.c b/src/util/src/terror.c index 27a08d8e9e..6cb508ebae 100644 --- a/src/util/src/terror.c +++ b/src/util/src/terror.c @@ -403,6 +403,8 @@ TAOS_DEFINE_ERROR(TSDB_CODE_HTTP_OP_TAG_VALUE_TOO_LONG, "tag value can not mor TAOS_DEFINE_ERROR(TSDB_CODE_HTTP_OP_VALUE_NULL, "value not find") TAOS_DEFINE_ERROR(TSDB_CODE_HTTP_OP_VALUE_TYPE, "value type should be boolean, number or string") +TAOS_DEFINE_ERROR(TSDB_CODE_HTTP_REQUEST_JSON_ERROR, "http request json error") + // odbc TAOS_DEFINE_ERROR(TSDB_CODE_ODBC_OOM, "out of memory") TAOS_DEFINE_ERROR(TSDB_CODE_ODBC_CONV_CHAR_NOT_NUM, "convertion not a valid literal input")