Skip to content

Commit

Permalink
add comments to validateListOrCountRequestForQuery to clarify usage o…
Browse files Browse the repository at this point in the history
…f sql statement (uber#2566)
  • Loading branch information
carlcortright authored and vancexu committed Sep 24, 2019
1 parent 7a4e0a4 commit efe8e19
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
11 changes: 7 additions & 4 deletions common/elasticsearch/validator/queryValidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,18 @@ func (qv *VisibilityQueryValidator) ValidateCountRequestForQuery(countRequest *w
// it also adds attr prefix for customized fields
func (qv *VisibilityQueryValidator) validateListOrCountRequestForQuery(whereClause string) (string, error) {
if len(whereClause) != 0 {
var sqlQuery string
// Build a placeholder query that allows us to easily parse the contents of the where clause.
// IMPORTANT: This query is never executed, it is just used to parse and validate whereClause
var placeholderQuery string
whereClause := strings.TrimSpace(whereClause)
// #nosec
if common.IsJustOrderByClause(whereClause) { // just order by
sqlQuery = "SELECT * FROM dummy " + whereClause
placeholderQuery = "SELECT * FROM dummy " + whereClause
} else {
sqlQuery = "SELECT * FROM dummy WHERE " + whereClause
placeholderQuery = "SELECT * FROM dummy WHERE " + whereClause
}

stmt, err := sqlparser.Parse(sqlQuery)
stmt, err := sqlparser.Parse(placeholderQuery)
if err != nil {
return "", &workflow.BadRequestError{Message: "Invalid query."}
}
Expand Down
5 changes: 5 additions & 0 deletions common/elasticsearch/validator/queryValidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,9 @@ func (s *queryValidatorSuite) TestValidateListRequestForQuery() {
query = "order by 123"
listRequest.Query = common.StringPtr(query)
s.Equal("BadRequestError{Message: invalid order by expression}", qv.ValidateListRequestForQuery(listRequest).Error())

// ensure sql injection doesn't work
query = "WorkflowID = 'wid'; SELECT * FROM important_table;"
listRequest.Query = common.StringPtr(query)
s.Equal("BadRequestError{Message: Invalid query.}", qv.ValidateListRequestForQuery(listRequest).Error())
}

0 comments on commit efe8e19

Please sign in to comment.