Add correct pagination for filters #335
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| WHERE database = '%s' | ||
| ORDER BY table, position | ||
| `, schema) | ||
|
|
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 5 days ago
To fix the problem, we should use parameterized queries or prepared statements to safely embed user input into SQL queries. This approach prevents SQL injection by ensuring that user input is treated as data rather than executable code.
In the provided code, we need to replace the use of fmt.Sprintf with parameterized queries using the QueryContext method. This involves modifying the getAllTableSchema and getTableSchema functions to use query parameters instead of directly embedding the schema and tableName values into the query string.
| @@ -105,3 +105,3 @@ | ||
| func getAllTableSchema(conn *sql.DB, schema string) (map[string][]engine.Record, error) { | ||
| query := fmt.Sprintf(` | ||
| query := ` | ||
| SELECT | ||
| @@ -111,7 +111,7 @@ | ||
| FROM system.columns | ||
| WHERE database = '%s' | ||
| WHERE database = ? | ||
| ORDER BY table, position | ||
| `, schema) | ||
| ` | ||
|
|
||
| rows, err := conn.QueryContext(context.Background(), query) | ||
| rows, err := conn.QueryContext(context.Background(), query, schema) | ||
| if err != nil { | ||
| @@ -134,3 +134,3 @@ | ||
| func getTableSchema(conn *sql.DB, schema string, tableName string) ([]engine.Record, error) { | ||
| query := fmt.Sprintf(` | ||
| query := ` | ||
| SELECT | ||
| @@ -139,7 +139,7 @@ | ||
| FROM system.columns | ||
| WHERE database = '%s' AND table = '%s' | ||
| WHERE database = ? AND table = ? | ||
| ORDER BY position | ||
| `, schema, tableName) | ||
| ` | ||
|
|
||
| rows, err := conn.QueryContext(context.Background(), query) | ||
| rows, err := conn.QueryContext(context.Background(), query, schema, tableName) | ||
| if err != nil { |
|
|
||
| countQuery := fmt.Sprintf("SELECT COUNT(*) %v", baseQuery) | ||
| var totalCount int | ||
| err = db.Raw(countQuery).Scan(&totalCount).Error |
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 5 days ago
To fix the problem, we need to replace the unsafe construction of SQL queries using fmt.Sprintf with parameterized queries. This involves using placeholders in the SQL query and passing the user-provided values as separate arguments to the query execution function. This approach ensures that the user-provided values are properly escaped and prevents SQL injection attacks.
In the core/src/plugins/mysql/mysql.go file, we need to:
- Replace the
fmt.Sprintfcalls with parameterized queries. - Pass the user-provided values as arguments to the
db.Rawfunction.
| @@ -119,10 +119,10 @@ | ||
|
|
||
| query := fmt.Sprintf(` | ||
| query := ` | ||
| SELECT TABLE_NAME, COLUMN_NAME, DATA_TYPE | ||
| FROM INFORMATION_SCHEMA.COLUMNS | ||
| WHERE TABLE_SCHEMA = '%v' | ||
| WHERE TABLE_SCHEMA = ? | ||
| ORDER BY TABLE_NAME, ORDINAL_POSITION | ||
| `, schema) | ||
| ` | ||
|
|
||
| if err := db.Raw(query).Scan(&result).Error; err != nil { | ||
| if err := db.Raw(query, schema).Scan(&result).Error; err != nil { | ||
| return nil, err | ||
| @@ -144,10 +144,11 @@ | ||
|
|
||
| baseQuery := fmt.Sprintf("FROM `%v`.`%s`", schema, storageUnit) | ||
| baseQuery := "FROM `?`.`?`" | ||
| args := []interface{}{schema, storageUnit} | ||
| if len(where) > 0 { | ||
| baseQuery = fmt.Sprintf("%v WHERE %v", baseQuery, where) | ||
| baseQuery += " WHERE " + where | ||
| } | ||
|
|
||
| countQuery := fmt.Sprintf("SELECT COUNT(*) %v", baseQuery) | ||
| countQuery := "SELECT COUNT(*) " + baseQuery | ||
| var totalCount int | ||
| err = db.Raw(countQuery).Scan(&totalCount).Error | ||
| err = db.Raw(countQuery, args...).Scan(&totalCount).Error | ||
| if err != nil { | ||
| @@ -156,5 +157,6 @@ | ||
|
|
||
| query := fmt.Sprintf("SELECT * %v LIMIT ? OFFSET ?", baseQuery) | ||
| query := "SELECT * " + baseQuery + " LIMIT ? OFFSET ?" | ||
| args = append(args, pageSize, pageOffset) | ||
|
|
||
| result, err := p.executeRawSQL(config, query, pageSize, pageOffset) | ||
| result, err := p.executeRawSQL(config, query, args...) | ||
| if err != nil { |
|
|
||
| countQuery := fmt.Sprintf("SELECT COUNT(*) %v", baseQuery) | ||
| var totalCount int | ||
| err = db.Raw(countQuery).Scan(&totalCount).Error |
Check failure
Code scanning / CodeQL
Database query built from user-controlled sources High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 5 days ago
To fix the problem, we need to use SQL parameterization instead of string concatenation to construct the SQL queries. This can be achieved by using placeholders (?) in the SQL query and passing the user-provided values as parameters to the db.Raw method. This approach ensures that the user-provided values are safely embedded into the query, preventing SQL injection attacks.
- Modify the
GetRowsmethod incore/src/plugins/postgres/postgres.goto use SQL parameterization. - Replace the
fmt.Sprintfcalls with parameterized queries. - Pass the user-provided values as parameters to the
db.Rawmethod.
| @@ -170,10 +170,10 @@ | ||
|
|
||
| baseQuery := fmt.Sprintf("FROM \"%v\".\"%s\"", schema, storageUnit) | ||
| baseQuery := "FROM ?.? " | ||
| if len(where) > 0 { | ||
| baseQuery = fmt.Sprintf("%v WHERE %v", baseQuery, where) | ||
| baseQuery += "WHERE " + where + " " | ||
| } | ||
|
|
||
| countQuery := fmt.Sprintf("SELECT COUNT(*) %v", baseQuery) | ||
| countQuery := "SELECT COUNT(*) " + baseQuery | ||
| var totalCount int | ||
| err = db.Raw(countQuery).Scan(&totalCount).Error | ||
| err = db.Raw(countQuery, schema, storageUnit).Scan(&totalCount).Error | ||
| if err != nil { | ||
| @@ -182,3 +182,3 @@ | ||
|
|
||
| query := fmt.Sprintf("SELECT * %v", baseQuery) | ||
| query := "SELECT * " + baseQuery | ||
| sortKeyRes, err := getPrimaryKeyColumns(db, schema, storageUnit) | ||
| @@ -186,8 +186,8 @@ | ||
| quotedKeys := common.JoinWithQuotes(sortKeyRes) | ||
| query = fmt.Sprintf("%v ORDER BY %v ASC", query, quotedKeys) | ||
| query += "ORDER BY " + quotedKeys + " ASC " | ||
| } | ||
|
|
||
| query = fmt.Sprintf("%v LIMIT ? OFFSET ?", query) | ||
| query += "LIMIT ? OFFSET ?" | ||
|
|
||
| result, err := p.executeRawSQL(config, query, pageSize, pageOffset) | ||
| result, err := p.executeRawSQL(config, query, schema, storageUnit, pageSize, pageOffset) | ||
| if err != nil { |
Fix: #281
Add total count for filter query to ensure that the pagination is correct.
Demo (shows filters has 3 pages with 15 total count on the filter):
