From 55feec34ea173604edcac67daede872a77370d47 Mon Sep 17 00:00:00 2001 From: Vishal Sharma Date: Fri, 24 Dec 2021 11:40:39 +0530 Subject: [PATCH] fix: db/logger security bugs (#558) --- pkg/query-service/app/clickhouseReader/reader.go | 12 ++++++------ pkg/query-service/app/parser.go | 13 +++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index d526ff8e22..30dd324e6f 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -1545,9 +1545,9 @@ func (r *ClickHouseReader) GetTags(ctx context.Context, serviceName string) (*[] tagItems := []model.TagItem{} - query := fmt.Sprintf(`SELECT DISTINCT arrayJoin(tagsKeys) as tagKeys FROM %s WHERE serviceName='%s' AND toDate(timestamp) > now() - INTERVAL 1 DAY`, r.indexTable, serviceName) + query := fmt.Sprintf(`SELECT DISTINCT arrayJoin(tagsKeys) as tagKeys FROM %s WHERE serviceName=? AND toDate(timestamp) > now() - INTERVAL 1 DAY`, r.indexTable) - err := r.db.Select(&tagItems, query) + err := r.db.Select(&tagItems, query, serviceName) zap.S().Info(query) @@ -1563,9 +1563,9 @@ func (r *ClickHouseReader) GetOperations(ctx context.Context, serviceName string operations := []string{} - query := fmt.Sprintf(`SELECT DISTINCT(name) FROM %s WHERE serviceName='%s' AND toDate(timestamp) > now() - INTERVAL 1 DAY`, r.indexTable, serviceName) + query := fmt.Sprintf(`SELECT DISTINCT(name) FROM %s WHERE serviceName=? AND toDate(timestamp) > now() - INTERVAL 1 DAY`, r.indexTable) - err := r.db.Select(&operations, query) + err := r.db.Select(&operations, query, serviceName) zap.S().Info(query) @@ -1580,9 +1580,9 @@ func (r *ClickHouseReader) SearchTraces(ctx context.Context, traceId string) (*[ var searchScanReponses []model.SearchSpanReponseItem - query := fmt.Sprintf("SELECT timestamp, spanID, traceID, serviceName, name, kind, durationNano, tagsKeys, tagsValues, references FROM %s WHERE traceID='%s'", r.indexTable, traceId) + query := fmt.Sprintf("SELECT timestamp, spanID, traceID, serviceName, name, kind, durationNano, tagsKeys, tagsValues, references FROM %s WHERE traceID=?", r.indexTable) - err := r.db.Select(&searchScanReponses, query) + err := r.db.Select(&searchScanReponses, query, traceId) zap.S().Info(query) diff --git a/pkg/query-service/app/parser.go b/pkg/query-service/app/parser.go index 99ce336f5f..2cf98542dd 100644 --- a/pkg/query-service/app/parser.go +++ b/pkg/query-service/app/parser.go @@ -7,6 +7,7 @@ import ( "math" "net/http" "strconv" + "strings" "time" promModel "github.com/prometheus/common/model" @@ -359,12 +360,18 @@ func parseSearchSpanAggregatesRequest(r *http.Request) (*model.SpanSearchAggrega operationName := r.URL.Query().Get("operation") if len(operationName) != 0 { params.OperationName = operationName + // Escaping new line chars to avoid CWE-117 + operationName = strings.Replace(operationName, "\n", "", -1) + operationName = strings.Replace(operationName, "\r", "", -1) zap.S().Debug("Operation Name: ", operationName) } kind := r.URL.Query().Get("kind") if len(kind) != 0 { params.Kind = kind + // Escaping new line chars to avoid CWE-117 + kind = strings.Replace(kind, "\n", "", -1) + kind = strings.Replace(kind, "\r", "", -1) zap.S().Debug("Kind: ", kind) } @@ -418,12 +425,18 @@ func parseSpanSearchRequest(r *http.Request) (*model.SpanSearchParams, error) { operationName := r.URL.Query().Get("operation") if len(operationName) != 0 { params.OperationName = operationName + // Escaping new line chars to avoid CWE-117 + operationName = strings.Replace(operationName, "\n", "", -1) + operationName = strings.Replace(operationName, "\r", "", -1) zap.S().Debug("Operation Name: ", operationName) } kind := r.URL.Query().Get("kind") if len(kind) != 0 { params.Kind = kind + // Escaping new line chars to avoid CWE-117 + kind = strings.Replace(kind, "\n", "", -1) + kind = strings.Replace(kind, "\r", "", -1) zap.S().Debug("Kind: ", kind) }