Skip to content

Commit eb125b9

Browse files
kartik-579kartik-579
authored
Merge pull request #6334 from devtron-labs/sql-injection-fix-2
fix: sql injection fixes
2 parents 1f5f76e + 4617e21 commit eb125b9

File tree

8 files changed

+50
-40
lines changed

8 files changed

+50
-40
lines changed

‎internal/sql/repository/AppListingRepository.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,10 @@ func (impl AppListingRepositoryImpl) FetchAppsByEnvironmentV2(appListingFilter h
308308

309309
// if any pipeline found get the latest deployment time
310310
if len(pipelineIds) > 0 {
311-
query := impl.appListingRepositoryQueryBuilder.BuildAppListingQueryLastDeploymentTimeV2(pipelineIds)
311+
query, queryParams := impl.appListingRepositoryQueryBuilder.BuildAppListingQueryLastDeploymentTimeV2(pipelineIds)
312312
impl.Logger.Debugw("basic app detail query: ", query)
313313
start := time.Now()
314-
_, err := impl.dbConnection.Query(&lastDeployedTimeDTO, query)
314+
_, err := impl.dbConnection.Query(&lastDeployedTimeDTO, query, queryParams...)
315315
middleware.AppListingDuration.WithLabelValues("buildAppListingQueryLastDeploymentTime", "devtron").Observe(time.Since(start).Seconds())
316316
if err != nil {
317317
impl.Logger.Errorw("error in getting latest deployment time for given pipelines", "err", err, "pipelines", pipelineIds, "query", query)

‎internal/sql/repository/CiArtifactRepository.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,8 @@ func (impl CiArtifactRepositoryImpl) GetArtifactsByCDPipelineV3(listingFilterOpt
362362
artifactsResp := make([]*CiArtifactWithExtraData, 0, listingFilterOpts.Limit)
363363
var artifacts []*CiArtifact
364364
totalCount := 0
365-
finalQuery := BuildQueryForParentTypeCIOrWebhook(*listingFilterOpts)
366-
_, err := impl.dbConnection.Query(&artifactsResp, finalQuery)
365+
finalQuery, finalQueryParams := BuildQueryForParentTypeCIOrWebhook(*listingFilterOpts)
366+
_, err := impl.dbConnection.Query(&artifactsResp, finalQuery, finalQueryParams...)
367367
if err != nil {
368368
return nil, totalCount, err
369369
}

‎internal/sql/repository/CiArtifactsListingQueryBuilder.go‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,28 @@ import (
2020
"fmt"
2121
"github.com/devtron-labs/devtron/api/bean"
2222
"github.com/devtron-labs/devtron/internal/sql/repository/helper"
23+
"github.com/go-pg/pg"
2324
)
2425

2526
const EmptyLikeRegex = "%%"
2627

27-
func BuildQueryForParentTypeCIOrWebhook(listingFilterOpts bean.ArtifactsListFilterOptions) string {
28-
commonPaginatedQueryPart := fmt.Sprintf(" cia.image LIKE '%v'", listingFilterOpts.SearchString)
28+
func BuildQueryForParentTypeCIOrWebhook(listingFilterOpts bean.ArtifactsListFilterOptions) (string, []interface{}) {
29+
commonPaginatedQueryPart, commonPaginatedQueryParams := " cia.image LIKE ?", []interface{}{listingFilterOpts.SearchString}
2930
orderByClause := " ORDER BY cia.id DESC"
30-
limitOffsetQueryPart := fmt.Sprintf(" LIMIT %v OFFSET %v", listingFilterOpts.Limit, listingFilterOpts.Offset)
31+
limitOffsetQueryPart, limitOffsetQueryParams := fmt.Sprintf(" LIMIT ? OFFSET ?"), []interface{}{listingFilterOpts.Limit, listingFilterOpts.Offset}
3132
finalQuery := ""
33+
var finalQueryParams []interface{}
34+
var remainingQueryParams []interface{}
3235
if listingFilterOpts.ParentStageType == bean.CI_WORKFLOW_TYPE {
3336
selectQuery := " SELECT cia.* "
3437
remainingQuery := " FROM ci_artifact cia" +
3538
" INNER JOIN ci_pipeline cp ON (cp.id=cia.pipeline_id or (cp.id=cia.component_id and cia.data_source='post_ci' ) )" +
36-
" INNER JOIN pipeline p ON (p.ci_pipeline_id = cp.id and p.id=%v )" +
39+
" INNER JOIN pipeline p ON (p.ci_pipeline_id = cp.id and p.id=? )" +
3740
" WHERE "
38-
remainingQuery = fmt.Sprintf(remainingQuery, listingFilterOpts.PipelineId)
41+
remainingQueryParams = []interface{}{listingFilterOpts.PipelineId}
3942
if len(listingFilterOpts.ExcludeArtifactIds) > 0 {
40-
remainingQuery += fmt.Sprintf("cia.id NOT IN (%s) AND ", helper.GetCommaSepratedString(listingFilterOpts.ExcludeArtifactIds))
43+
remainingQuery += "cia.id NOT IN (?) AND "
44+
remainingQueryParams = append(remainingQueryParams, pg.In(listingFilterOpts.ExcludeArtifactIds))
4145
}
4246

4347
countQuery := " SELECT count(cia.id) as total_count"
@@ -47,19 +51,24 @@ func BuildQueryForParentTypeCIOrWebhook(listingFilterOpts bean.ArtifactsListFilt
4751
} else if listingFilterOpts.ParentStageType == bean.WEBHOOK_WORKFLOW_TYPE {
4852
selectQuery := " SELECT cia.* "
4953
remainingQuery := " FROM ci_artifact cia " +
50-
" WHERE cia.external_ci_pipeline_id = %v AND "
51-
remainingQuery = fmt.Sprintf(remainingQuery, listingFilterOpts.ParentId)
54+
" WHERE cia.external_ci_pipeline_id = ? AND "
55+
remainingQueryParams = []interface{}{listingFilterOpts.ParentId}
5256
if len(listingFilterOpts.ExcludeArtifactIds) > 0 {
53-
remainingQuery += fmt.Sprintf("cia.id NOT IN (%s) AND ", helper.GetCommaSepratedString(listingFilterOpts.ExcludeArtifactIds))
57+
remainingQuery += "cia.id NOT IN (?) AND "
58+
remainingQueryParams = append(remainingQueryParams, pg.In(listingFilterOpts.ExcludeArtifactIds))
5459
}
5560

5661
countQuery := " SELECT count(cia.id) as total_count"
5762
totalCountQuery := countQuery + remainingQuery + commonPaginatedQueryPart
5863
selectQuery = fmt.Sprintf("%s,(%s) ", selectQuery, totalCountQuery)
5964
finalQuery = selectQuery + remainingQuery + commonPaginatedQueryPart + orderByClause + limitOffsetQueryPart
60-
6165
}
62-
return finalQuery
66+
finalQueryParams = append(finalQueryParams, remainingQueryParams...)
67+
finalQueryParams = append(finalQueryParams, commonPaginatedQueryParams...)
68+
finalQueryParams = append(finalQueryParams, remainingQueryParams...)
69+
finalQueryParams = append(finalQueryParams, commonPaginatedQueryParams...)
70+
finalQueryParams = append(finalQueryParams, limitOffsetQueryParams...)
71+
return finalQuery, finalQueryParams
6372
}
6473

6574
func BuildQueryForArtifactsForCdStage(listingFilterOptions bean.ArtifactsListFilterOptions) string {

‎internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,17 @@ func (impl AppListingRepositoryQueryBuilder) TestForCommonAppFilter(appListingFi
160160
return query, queryParams
161161
}
162162

163-
func (impl AppListingRepositoryQueryBuilder) BuildAppListingQueryLastDeploymentTimeV2(pipelineIDs []int) string {
163+
func (impl AppListingRepositoryQueryBuilder) BuildAppListingQueryLastDeploymentTimeV2(pipelineIDs []int) (string, []interface{}) {
164164
whereCondition := ""
165+
queryParams := []interface{}{}
165166
if len(pipelineIDs) > 0 {
166-
whereCondition += fmt.Sprintf(" Where pco.pipeline_id IN (%s) ", GetCommaSepratedString(pipelineIDs))
167+
whereCondition += " Where pco.pipeline_id IN (?) "
168+
queryParams = append(queryParams, pg.In(pipelineIDs))
167169
}
168170
query := "select pco.pipeline_id , MAX(pco.created_on) as last_deployed_time" +
169171
" from pipeline_config_override pco" + whereCondition +
170172
" GROUP BY pco.pipeline_id;"
171-
return query
173+
return query, queryParams
172174
}
173175

174176
func (impl AppListingRepositoryQueryBuilder) GetAppIdsQueryWithPaginationForLastDeployedSearch(appListingFilter AppListingFilter) (string, []interface{}) {

‎internal/sql/repository/pipelineConfig/CdWorfkflowRepository.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package pipelineConfig
1919
import (
2020
"context"
2121
"errors"
22-
"fmt"
2322
apiBean "github.com/devtron-labs/devtron/api/bean"
2423
"github.com/devtron-labs/devtron/client/gitSensor"
2524
"github.com/devtron-labs/devtron/internal/sql/repository"
@@ -539,10 +538,9 @@ func (impl *CdWorkflowRepositoryImpl) FindBasicWorkflowRunnerById(wfrId int) (*C
539538

540539
func (impl *CdWorkflowRepositoryImpl) FindRetriedWorkflowCountByReferenceId(wfrId int) (int, error) {
541540
retryCount := 0
542-
query := fmt.Sprintf("select count(id) "+
543-
"from cd_workflow_runner where ref_cd_workflow_runner_id = %v", wfrId)
541+
query := "select count(id) from cd_workflow_runner where ref_cd_workflow_runner_id = ?"
544542

545-
_, err := impl.dbConnection.Query(&retryCount, query)
543+
_, err := impl.dbConnection.Query(&retryCount, query, wfrId)
546544
return retryCount, err
547545
}
548546

‎internal/sql/repository/pipelineConfig/CiWorkflowRepository.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
package pipelineConfig
1818

1919
import (
20-
"fmt"
2120
"github.com/devtron-labs/devtron/internal/sql/constants"
22-
"github.com/devtron-labs/devtron/internal/sql/repository/helper"
2321
"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig/bean/workflow"
2422
"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig/bean/workflow/cdWorkflow"
2523
"github.com/go-pg/pg"
@@ -236,10 +234,9 @@ func (impl *CiWorkflowRepositoryImpl) FindById(id int) (*CiWorkflow, error) {
236234

237235
func (impl *CiWorkflowRepositoryImpl) FindRetriedWorkflowCountByReferenceId(id int) (int, error) {
238236
retryCount := 0
239-
query := fmt.Sprintf("select count(*) "+
240-
"from ci_workflow where ref_ci_workflow_id = %v", id)
237+
query := "select count(*) from ci_workflow where ref_ci_workflow_id = ?"
241238

242-
_, err := impl.dbConnection.Query(&retryCount, query)
239+
_, err := impl.dbConnection.Query(&retryCount, query, id)
243240
return retryCount, err
244241
}
245242

@@ -339,9 +336,8 @@ func (impl *CiWorkflowRepositoryImpl) FindLastTriggeredWorkflowGitTriggersByArti
339336
}
340337
query := "SELECT cw.git_triggers,cw.id,cw.triggered_by,cw.ci_pipeline_id,cia.id as ci_artifact_id" +
341338
" FROM ci_workflow cw INNER JOIN ci_artifact cia on cia.ci_workflow_id = cw.id " +
342-
" WHERE cia.id IN (%s)"
343-
query = fmt.Sprintf(query, helper.GetCommaSepratedString(ciArtifactIds))
344-
_, err := impl.dbConnection.Query(&workflows, query)
339+
" WHERE cia.id IN (?)"
340+
_, err := impl.dbConnection.Query(&workflows, query, pg.In(ciArtifactIds))
345341
return workflows, err
346342
}
347343

‎pkg/auth/user/repository/UserRepository.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,10 @@ func (impl UserRepositoryImpl) GetAllExecutingQuery(query string, queryParams []
181181
func (impl UserRepositoryImpl) FetchActiveUserByEmail(email string) (bean.UserInfo, error) {
182182
var users bean.UserInfo
183183

184+
emailSearchQuery, queryParams := helper.GetEmailSearchQuery("u", email)
184185
query := fmt.Sprintf("SELECT u.id, u.email_id, u.access_token, u.user_type FROM users u"+
185-
" WHERE u.active = true and %s order by u.updated_on desc", helper.GetEmailSearchQuery("u", email))
186-
_, err := impl.dbConnection.Query(&users, query, email)
186+
" WHERE u.active = true and %s order by u.updated_on desc", emailSearchQuery)
187+
_, err := impl.dbConnection.Query(&users, query, queryParams...)
187188
if err != nil {
188189
impl.Logger.Errorw("Exception caught:", "err", err)
189190
return users, err
@@ -197,12 +198,13 @@ func (impl UserRepositoryImpl) FetchUserDetailByEmail(email string) (bean.UserIn
197198
var users []bean.UserRole
198199
var userFinal bean.UserInfo
199200

201+
emailSearchQuery, queryParams := helper.GetEmailSearchQuery("u", email)
200202
query := fmt.Sprintf("SELECT u.id, u.email_id, u.user_type, r.role FROM users u"+
201203
" INNER JOIN user_roles ur ON ur.user_id=u.id"+
202204
" INNER JOIN roles r ON r.id=ur.role_id"+
203205
" WHERE %s and u.active = true"+
204-
" ORDER BY u.updated_on desc;", helper.GetEmailSearchQuery("u", email))
205-
_, err := impl.dbConnection.Query(&users, query, email)
206+
" ORDER BY u.updated_on desc;", emailSearchQuery)
207+
_, err := impl.dbConnection.Query(&users, query, queryParams...)
206208
if err != nil {
207209
return userFinal, err
208210
}

‎pkg/auth/user/repository/helper/UserRepositoryQueryBuilder.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,11 @@ func GetQueryForGroupListingWithFilters(req *bean.ListingRequest) (string, []int
8484

8585
orderCondition := ""
8686
if len(req.SortBy) > 0 && !req.CountCheck {
87-
orderCondition += fmt.Sprintf(" order by %s ", req.SortBy)
87+
orderCondition += " order by ? "
88+
queryParams = append(queryParams, req.SortBy)
8889
if req.SortOrder == bean2.Desc {
89-
orderCondition += fmt.Sprintf(" %s ", bean2.Desc)
90+
orderCondition += " ? "
91+
queryParams = append(queryParams, bean2.Desc)
9092
}
9193
}
9294
if req.Size > 0 && !req.CountCheck && !req.ShowAll {
@@ -103,9 +105,10 @@ func GetQueryForGroupListingWithFilters(req *bean.ListingRequest) (string, []int
103105

104106
}
105107

106-
func GetEmailSearchQuery(usersTableAlias string, emailId string) string {
108+
func GetEmailSearchQuery(usersTableAlias string, emailId string) (string, []interface{}) {
109+
queryParams := []interface{}{emailId, emailId}
107110
expression := fmt.Sprintf(
108-
"( (%s.user_type is NULL and %s.email_id ILIKE '%s' ) or (%s.user_type='apiToken' and %s.email_id='%s') )",
109-
usersTableAlias, usersTableAlias, emailId, usersTableAlias, usersTableAlias, emailId)
110-
return expression
111+
"( (%s.user_type is NULL and %s.email_id ILIKE ? ) or (%s.user_type='apiToken' and %s.email_id=?) )",
112+
usersTableAlias, usersTableAlias, usersTableAlias, usersTableAlias)
113+
return expression, queryParams
111114
}

0 commit comments

Comments
 (0)