Input Validation / Potential query construction injection in CloudWatch Logs queries

PoC (conceptual exploit path): Assumptions: - Grafana version prior to this commit allowed configuring CloudWatch Logs data queries with a dataSources field (logDataSources) that is concatenated into the final CloudWatch Logs query string. - The final query string could be crafted from user-supplied data source names/types and injected into the CWLI/PPL/SQL query path. Attack scenario (pre-fix risk): 1) An attacker with access to a Grafana CloudWatch Logs query configuration creates a data source entry: { "name": "evil_name; DROP TABLE users;--", "type": "flow" } or a name containing a newline or pipe character. 2) This value is incorporated into the generated start query (source clause) that precedes the actual query body, potentially allowing the attacker to terminate the injected source clause and inject additional query text or malformed syntax into the CloudWatch Logs query string. 3) When the query is executed against CloudWatch, the injected content could cause parsing errors, unexpected behavior, or, in the worst case, unintended query semantics depending on how the final QueryString is constructed. What the fix changes (post-fix): - Validates that both name and type are non-empty and conform to a safe pattern (allowed characters only, e.g., A-Za-z0-9_-). - Enforces uniqueness of data source identifiers and caps the number of data sources to 10. - Returns downstream errors when validation fails, preventing malformed query strings from being constructed or sent to CloudWatch. How to test (after applying the fix): - Try to configure a CloudWatch Logs query with logDataSources where name or type contain disallowed characters (e.g., newline, semicolon, or spaces). - Observe that the API rejects the request with a validation error rather than constructing a potentially vulnerable QueryString. - Verify that valid data sources (matching the allowed pattern) are accepted and the resulting query is constructed safely.

diff --git a/packages/grafana-schema/src/raw/composable/cloudwatch/dataquery/x/types.gen.ts b/packages/grafana-schema/src/raw/composable/cloudwatch/dataquery/x/types.gen.ts index 726f41bfb9012..02b51c6e44527 100644 --- a/packages/grafana-schema/src/raw/composable/cloudwatch/dataquery/x/types.gen.ts +++ b/packages/grafana-schema/src/raw/composable/cloudwatch/dataquery/x/types.gen.ts @@ -250,6 +250,10 @@ export interface CloudWatchLogsQuery extends common.DataQuery { */ expression?: string; id: string; + /** + * Data sources to query + */ + logDataSources?: Array<LogDataSource>; /** * Log group class filter for namePrefix and allLogGroups scope modes */ @@ -297,6 +301,7 @@ export interface CloudWatchLogsQuery extends common.DataQuery { } export const defaultCloudWatchLogsQuery: Partial<CloudWatchLogsQuery> = { + logDataSources: [], logGroupNames: [], logGroupPrefixes: [], logGroups: [], @@ -350,6 +355,17 @@ export interface LogGroup { name: string; } +export interface LogDataSource { + /** + * Name of the data source + */ + name: string; + /** + * Type of the data source + */ + type: string; +} + /** * Shape of a CloudWatch Annotation query */ diff --git a/pkg/tsdb/cloudwatch/data_sources_test.go b/pkg/tsdb/cloudwatch/data_sources_test.go new file mode 100644 index 0000000000000..2ebd1c63b0899 --- /dev/null +++ b/pkg/tsdb/cloudwatch/data_sources_test.go @@ -0,0 +1,77 @@ +package cloudwatch + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/services" +) + +func TestDataSourcesRoute(t *testing.T) { + origDataSourcesService := services.NewDataSourcesService + t.Cleanup(func() { + services.NewDataSourcesService = origDataSourcesService + }) + + var mockDataSourcesService = mocks.DataSourcesService{} + services.NewDataSourcesService = func(models.CloudWatchLogsAPIProvider, bool) models.DataSourcesProvider { + return &mockDataSourcesService + } + + t.Run("successfully returns data sources", func(t *testing.T) { + mockDataSourcesService = mocks.DataSourcesService{} + mockDataSourcesService.On("GetDataSources", mock.Anything).Return([]resources.ResourceResponse[resources.LogDataSource]{{ + Value: resources.LogDataSource{ + Name: "amazon_vpc", + Type: "flow", + }, + }}, nil) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/data-sources", nil) + ds := newTestDatasource() + handler := http.HandlerFunc(ds.resourceRequestMiddleware(ds.DataSourcesHandler)) + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.JSONEq(t, `[{"value":{"name":"amazon_vpc","type":"flow"}}]`, rr.Body.String()) + }) + + t.Run("passes pattern from query parameter", func(t *testing.T) { + mockDataSourcesService = mocks.DataSourcesService{} + mockDataSourcesService.On("GetDataSources", mock.Anything).Return([]resources.ResourceResponse[resources.LogDataSource]{}, nil) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/data-sources?pattern=amazon", nil) + ds := newTestDatasource() + handler := http.HandlerFunc(ds.resourceRequestMiddleware(ds.DataSourcesHandler)) + handler.ServeHTTP(rr, req) + + mockDataSourcesService.AssertCalled(t, "GetDataSources", resources.DataSourcesRequest{ + Pattern: new("amazon"), + }) + }) + + t.Run("returns error if service returns error", func(t *testing.T) { + mockDataSourcesService = mocks.DataSourcesService{} + mockDataSourcesService.On("GetDataSources", mock.Anything). + Return([]resources.ResourceResponse[resources.LogDataSource]{}, fmt.Errorf("some error")) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/data-sources", nil) + ds := newTestDatasource() + handler := http.HandlerFunc(ds.resourceRequestMiddleware(ds.DataSourcesHandler)) + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusInternalServerError, rr.Code) + assert.JSONEq(t, `{"Error":"some error","Message":"GetDataSources error: some error","StatusCode":500}`, rr.Body.String()) + }) +} diff --git a/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go b/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go index faf969724838a..b7a338a40de5e 100644 --- a/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go +++ b/pkg/tsdb/cloudwatch/kinds/dataquery/types_dataquery_gen.go @@ -334,6 +334,8 @@ type CloudWatchLogsQuery struct { LogGroups []LogGroup `json:"logGroups,omitempty"` // @deprecated use logGroups LogGroupNames []string `json:"logGroupNames,omitempty"` + // Data sources to query + LogDataSources []LogDataSource `json:"logDataSources,omitempty"` // Language used for querying logs, can be CWLI, SQL, or PPL. If empty, the default language is CWLI. QueryLanguage *LogsQueryLanguage `json:"queryLanguage,omitempty"` // Log group selection scope - determines how log groups are selected for the query @@ -381,6 +383,18 @@ func NewLogGroup() *LogGroup { return &LogGroup{} } +type LogDataSource struct { + // Name of the data source + Name string `json:"name"` + // Type of the data source + Type string `json:"type"` +} + +// NewLogDataSource creates a new LogDataSource object. +func NewLogDataSource() *LogDataSource { + return &LogDataSource{} +} + // Shape of a Cloudwatch Log Anomalies query type CloudWatchLogsAnomaliesQuery struct { Id string `json:"id"` diff --git a/pkg/tsdb/cloudwatch/log_actions.go b/pkg/tsdb/cloudwatch/log_actions.go index d4b5086d584f7..c15614c18372b 100644 --- a/pkg/tsdb/cloudwatch/log_actions.go +++ b/pkg/tsdb/cloudwatch/log_actions.go @@ -30,6 +30,7 @@ const ( logIdentifierInternal = "__log__grafana_internal__" logStreamIdentifierInternal = "__logstream__grafana_internal__" logGroupsMacro = "$__logGroups" + sourceMacro = "$__source" // Only for CWLI queries. // The fields @log and @logStream are always included in the results of a user's query @@ -43,9 +44,12 @@ const ( maxSourceLogGroupPrefixes = 5 minSourceLogGroupPrefixLen = 3 maxSourceAccountIdentifiers = 20 + maxDataSourceSelections = 10 ) var sourceCommandRegex = regexp.MustCompile(`(?i)^\s*source\s+`) +var pplSourceCommandRegex = regexp.MustCompile(`(?im)(^|\|)\s*(--[^\n]*(\n|$)\s*)*(search\s+)?source\s*=`) +var dataSourceIdentifierPartRegex = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) type AWSError struct { Code string @@ -53,6 +57,15 @@ type AWSError struct { Payload map[string]string } +type startQueryPlan struct { + queryString string + // useStartQueryInputLogGroups controls whether executeStartQuery should populate + // StartQueryInput.LogGroupNames / LogGroupIdentifiers. Queries that keep all log + // group selection inside the query text, such as SOURCE-injected CWLI scopes and + // SQL/PPL source syntax, leave this false. + useStartQueryInputLogGroups bool +} + func (e *AWSError) Error() string { return fmt.Sprintf("CloudWatch error: %s: %s", e.Code, e.Message) } @@ -220,15 +233,15 @@ func (ds *DataSource) executeStartQuery(ctx context.Context, logsClient models.C } logGroupIdentifiers := buildLogGroupIdentifiers(logsQuery.LogGroups, isMonitoringAccount) - - isCWLIQuery := *logsQuery.QueryLanguage == dataquery.LogsQueryLanguageCWLI - finalQueryString, usesSourceCommand, err := buildFinalQueryString(logsQuery, isMonitoringAccount, isCWLIQuery) + dataSourceIdentifiers, err := buildDataSourceIdentifiers(logsQuery.LogDataSources) if err != nil { - return nil, err + return nil, backend.DownstreamError(err) + } + if len(dataSourceIdentifiers) > maxDataSourceSelections { + return nil, backend.DownstreamError(fmt.Errorf("maximum of %d data sources allowed, got %d", maxDataSourceSelections, len(dataSourceIdentifiers))) } - // Expand $__logGroups macro for SQL queries - finalQueryString, err = expandLogGroupsMacro(*logsQuery.QueryLanguage, finalQueryString, logGroupIdentifiers) + queryPlan, err := buildStartQueryPlan(logsQuery, isMonitoringAccount, logGroupIdentifiers, dataSourceIdentifiers) if err != nil { return nil, err } @@ -241,23 +254,11 @@ func (ds *DataSource) executeStartQuery(ctx context.Context, logsClient models.C // and also a little bit more but as CW logs accept only seconds as integers there is not much to do about // that. EndTime: aws.Int64(int64(math.Ceil(float64(endTime.UnixNano()) / 1e9))), - QueryString: aws.String(finalQueryString), + QueryString: aws.String(queryPlan.queryString), } - // When using SOURCE command (namePrefix or allLogGroups mode), log groups are specified - // in the query string, so we should NOT set LogGroupNames or LogGroupIdentifiers - // log group identifiers can be left out if the query is an SQL query or uses SOURCE command - if *logsQuery.QueryLanguage != dataquery.LogsQueryLanguageSQL && !usesSourceCommand { - useLogGroupIdentifiers := len(logsQuery.LogGroups) > 0 && isMonitoringAccount - if useLogGroupIdentifiers { - startQueryInput.LogGroupIdentifiers = logGroupIdentifiers - } else { - // even though logsQuery.LogGroupNames is deprecated, we still need to support it for backwards compatibility and alert queries - startQueryInput.LogGroupNames = append([]string(nil), logsQuery.LogGroupNames...) - if len(startQueryInput.LogGroupNames) == 0 && len(logGroupIdentifiers) > 0 { - startQueryInput.LogGroupNames = logGroupIdentifiers - } - } + if queryPlan.useStartQueryInputLogGroups { + applyStartQueryLogGroupSelection(startQueryInput, logsQuery, logGroupIdentifiers, isMonitoringAccount) } if logsQuery.Limit != nil { @@ -316,50 +317,278 @@ func buildLogGroupIdentifiers(logGroups []dataquery.LogGroup, isMonitoringAccoun return logGroupIdentifiers } -func buildFinalQueryString(logsQuery models.LogsQuery, isMonitoringAccount bool, isCWLIQuery bool) (string, bool, error) { - finalQueryString := logsQuery.QueryString - if !isCWLIQuery { - return finalQueryString, false, nil +func buildDataSourceIdentifiers(dataSources []dataquery.LogDataSource) ([]string, error) { + if len(dataSources) == 0 { + return nil, nil } + seen := make(map[string]struct{}, len(dataSources)) + identifiers := make([]string, 0, len(dataSources)) + + for _, ds := range dataSources { + if ds.Name == "" || ds.Type == "" { + return nil, fmt.Errorf("data source selection must include both name and type") + } + if err := validateDataSourceIdentifierPart("name", ds.Name); err != nil { + return nil, err + } + if err := validateDataSourceIdentifierPart("type", ds.Type); err != nil { + return nil, err + } + identifier := fmt.Sprintf("%s.%s", ds.Name, ds.Type) + if _, exists := seen[identifier]; exists { + continue + } + seen[identifier] = struct{}{} + identifiers = append(identifiers, identifier) + } + + return identifiers, nil +} + +func buildStartQueryPlan( + logsQuery models.LogsQuery, + isMonitoringAccount bool, + logGroupIdentifiers []string, + dataSourceIdentifiers []string, +) (startQueryPlan, error) { + switch *logsQuery.QueryLanguage { + case dataquery.LogsQueryLanguageCWLI: + return buildCWLIStartQuery(logsQuery, isMonitoringAccount, logGroupIdentifiers, dataSourceIdentifiers) + case dataquery.LogsQueryLanguagePPL: + return buildPPLStartQuery(logsQuery, logGroupIdentifiers, dataSourceIdentifiers) + case dataquery.LogsQueryLanguageSQL: + return buildSQLStartQuery(logsQuery, logGroupIdentifiers, dataSourceIdentifiers) + default: + return startQueryPlan{ + queryString: logsQuery.QueryString, + useStartQueryInputLogGroups: true, + }, nil + } +} + +func buildCWLIStartQuery( + logsQuery models.LogsQuery, + isMonitoringAccount bool, + logGroupIdentifiers []string, + dataSourceIdentifiers []string, +) (startQueryPlan, error) { usesNamePrefixScope := logsQuery.LogsQueryScope != nil && *logsQuery.LogsQueryScope == dataquery.LogsQueryScopeNamePrefix usesAllLogGroupsScope := logsQuery.LogsQueryScope != nil && *logsQuery.LogsQueryScope == dataquery.LogsQueryScopeAllLogGroups - usesSourceCommand := usesNamePrefixScope || usesAllLogGroupsScope + usesDirectLogGroupScope := !usesNamePrefixScope && !usesAllLogGroupsScope + usesExactLogGroupSelection := usesDirectLogGroupScope && len(logGroupIdentifiers) > 0 + useStartQueryInputLogGroups := usesDirectLogGroupScope && (len(dataSourceIdentifiers) == 0 || usesExactLogGroupSelection) - if usesSourceCommand { - if containsSourceCommand(logsQuery.QueryString) { - return "", false, backend.DownstreamError(fmt.Errorf("query cannot contain SOURCE command when using Name prefix or All log groups mode")) - } + if usesDirectLogGroupScope && len(dataSourceIdentifiers) == 0 { + return startQueryPlan{ + queryString: logContextFieldsClause + "|" + logsQuery.QueryString, + useStartQueryInputLogGroups: useStartQueryInputLogGroups, + }, nil + } - if usesNamePrefixScope { - if err := validateLogGroupPrefixes(logsQuery.LogGroupPrefixes); err != nil { - return "", false, backend.DownstreamError(err) - } + if containsSourceCommand(logsQuery.QueryString) { + return startQueryPlan{}, backend.DownstreamError(fmt.Errorf("query cannot contain SOURCE command when SOURCE is injected automatically")) + } + + if usesNamePrefixScope { + if err := validateLogGroupPrefixes(logsQuery.LogGroupPrefixes); err != nil { + return startQueryPlan{}, backend.DownstreamError(err) } + } + if usesNamePrefixScope || usesAllLogGroupsScope { if err := validateAccountIdentifiers(logsQuery.SelectedAccountIds); err != nil { - return "", false, backend.DownstreamError(err) + return startQueryPlan{}, backend.DownstreamError(err) } + } - includeAccounts := isMonitoringAccount && len(logsQuery.SelectedAccountIds) > 0 + includeAccounts := isMonitoringAccount && (usesNamePrefixScope || usesAllLogGroupsScope) && len(logsQuery.SelectedAccountIds) > 0 - sourceClause := buildSourceClause(logsQuery, includeAccounts) - return sourceClause + " | " + logContextFieldsClause + "|" + logsQuery.QueryString, true, nil + sourceLogGroupIdentifiers := logGroupIdentifiers + if usesExactLogGroupSelection && len(dataSourceIdentifiers) > 0 { + sourceLogGroupIdentifiers = nil } - finalQueryString = logContextFieldsClause + "|" + finalQueryString - return finalQueryString, false, nil + sourceClause := buildSourceClause(logsQuery, includeAccounts, sourceLogGroupIdentifiers, dataSourceIdentifiers) + return startQueryPlan{ + queryString: sourceClause + " | " + logContextFieldsClause + "|" + logsQuery.QueryString, + useStartQueryInputLogGroups: useStartQueryInputLogGroups, + }, nil } -func expandLogGroupsMacro(queryLanguage dataquery.LogsQueryLanguage, queryString string, logGroupIden ... [truncated]