Skip to content

Commit 6e7b5f1

Browse files
warren830tamas-la
authored andcommitted
fix: add SQL identifier validation to prevent SQL injection via table/column names (#8769)
Add ValidateTableName and ValidateColumnName functions in core/dal to ensure table and column names used in dynamic SQL are safe identifiers. Applied to scope_service_helper, scope_generic_helper, and customized_fields_extractor.
1 parent d06e6b9 commit 6e7b5f1

4 files changed

Lines changed: 71 additions & 3 deletions

File tree

backend/core/dal/identifier.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package dal
19+
20+
import (
21+
"fmt"
22+
"regexp"
23+
24+
"github.com/apache/incubator-devlake/core/errors"
25+
)
26+
27+
// validIdentifierRegex matches valid SQL identifiers: alphanumeric, underscores, and dots (for schema.table)
28+
var validIdentifierRegex = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_.]*$`)
29+
30+
// ValidateTableName checks that a table name is a safe SQL identifier to prevent SQL injection.
31+
func ValidateTableName(name string) errors.Error {
32+
if name == "" {
33+
return errors.Default.New("table name must not be empty")
34+
}
35+
if !validIdentifierRegex.MatchString(name) {
36+
return errors.Default.New(fmt.Sprintf("invalid table name: %q", name))
37+
}
38+
return nil
39+
}
40+
41+
// ValidateColumnName checks that a column name is a safe SQL identifier to prevent SQL injection.
42+
func ValidateColumnName(name string) errors.Error {
43+
if name == "" {
44+
return errors.Default.New("column name must not be empty")
45+
}
46+
if !validIdentifierRegex.MatchString(name) {
47+
return errors.Default.New(fmt.Sprintf("invalid column name: %q", name))
48+
}
49+
return nil
50+
}

backend/helpers/pluginhelper/api/scope_generic_helper.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,9 @@ func (gs *GenericScopeApiHelper[Conn, Scope, ScopeConfig]) transactionalDelete(t
565565
}
566566
tx := gs.db.Begin()
567567
for _, table := range tables {
568+
if err := dal.ValidateTableName(table); err != nil {
569+
return errors.Default.Wrap(err, fmt.Sprintf("unsafe table name %q when deleting scope data", table))
570+
}
568571
where, params := generateWhereClause(table)
569572
gs.log.Info("deleting data from table %s with WHERE \"%s\" and params: \"%v\"", table, where, params)
570573
sql := fmt.Sprintf("DELETE FROM %s WHERE %s", table, where)

backend/helpers/srvhelper/scope_service_helper.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,9 @@ func (scopeSrv *ScopeSrvHelper[C, S, SC]) deleteScopeData(scope plugin.ToolLayer
255255
}
256256
tables := errors.Must1(scopeSrv.getAffectedTables())
257257
for _, table := range tables {
258+
if err := dal.ValidateTableName(table); err != nil {
259+
panic(errors.Default.Wrap(err, fmt.Sprintf("unsafe table name %q when deleting scope data", table)))
260+
}
258261
where, params := generateWhereClause(table)
259262
scopeSrv.log.Info("deleting data from table %s with WHERE \"%s\" and params: \"%v\"", table, where, params)
260263
sql := fmt.Sprintf("DELETE FROM %s WHERE %s", table, where)

backend/plugins/customize/tasks/customized_fields_extractor.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ func extractCustomizedFields(ctx context.Context, d dal.Dal, table, rawTable, ra
149149
// remove columns that are not primary key
150150
delete(row, "_raw_data_id")
151151
delete(row, "data")
152-
query, params := mkUpdate(table, updates, row)
152+
query, params, err := mkUpdate(table, updates, row)
153+
if err != nil {
154+
return err
155+
}
153156
err = d.Exec(query, params...)
154157
if err != nil {
155158
return errors.Default.Wrap(err, "Exec SQL error")
@@ -169,18 +172,27 @@ func fillInUpdates(result gjson.Result, field string, updates map[string]interfa
169172
}
170173

171174
// mkUpdate generates SQL statement and parameters for updating a record
172-
func mkUpdate(table string, updates map[string]interface{}, pk map[string]interface{}) (string, []interface{}) {
175+
func mkUpdate(table string, updates map[string]interface{}, pk map[string]interface{}) (string, []interface{}, error) {
176+
if err := dal.ValidateTableName(table); err != nil {
177+
return "", nil, err
178+
}
173179
var params []interface{}
174180
stat := fmt.Sprintf("UPDATE %s SET ", table)
175181
var uu []string
176182
for field, value := range updates {
183+
if err := dal.ValidateColumnName(field); err != nil {
184+
return "", nil, err
185+
}
177186
uu = append(uu, fmt.Sprintf("%s = ?", field))
178187
params = append(params, value)
179188
}
180189
var ww []string
181190
for field, value := range pk {
191+
if err := dal.ValidateColumnName(field); err != nil {
192+
return "", nil, err
193+
}
182194
ww = append(ww, fmt.Sprintf("%s = ?", field))
183195
params = append(params, value)
184196
}
185-
return stat + strings.Join(uu, ", ") + " WHERE " + strings.Join(ww, " AND "), params
197+
return stat + strings.Join(uu, ", ") + " WHERE " + strings.Join(ww, " AND "), params, nil
186198
}

0 commit comments

Comments
 (0)