Release - Fix path traversal bug for sqlite, new clickhouse options, and support for OpenAI-like URLs #329

modelorona · 2025-01-27T17:35:52Z

✨ Improvements

Added new clickhouse options for HTTP

Thanks to the contribution by @Slach, we've got additional options for clickhouse support, HTTP protocol, TLS mode, readonly and debug.

Support for OpenAI-like URLs

Thanks to the contribution by @learningpro, we support OpenAI-like URLs now. Check the readme to see how to get started!

🐛 Bug Fixes

Path traversal issue for sqlite

Resolved an issue with how the database location for sqlite was being parsed resulting in the possibility of path traversal. Thanks to @nnse for catching that one!

Parameter injection issue for MySQL

Resolved an issue where it was possible to inject parameters using the UI. Alongside this, we took the time to go through all of the other connectors and check how we're handling input. Please let us know if you find anything out of place! Thanks to @nnse for catching that one!

Thank you to everyone who contributed to this release! 🚀
Your feedback and support are invaluable.

…e, readonly and debug advanced options Signed-off-by: Slach <[email protected]>

add additional options for clickhouse support

Add OpenAI Compatible Provider

…n instead of string concat

… to config

…ake value as is

…search to build url using url.URL, further url.QueryEscapes

fix for sqlite path traversal

core/src/plugins/clickhouse/add.go

@@ -72,7 +72,7 @@
 		query += fmt.Sprintf("\nSETTINGS %s", strings.Join(settingsClauses, ", "))
 	}

-	err = conn.Exec(context.Background(), query)
+	_, err = conn.ExecContext(context.Background(), query)


To fix the problem, we need to ensure that user input is safely embedded into the SQL query. This can be achieved by using parameterized queries or prepared statements. In this case, we will use parameterized queries to avoid SQL injection vulnerabilities.

Modify the AddStorageUnit function to use parameterized queries for the schema and storageUnit parameters.

Use sql.Named to safely include the schema and storageUnit parameters in the query.

core/src/plugins/clickhouse/add.go

@@ -100,6 +100,6 @@
 	query := fmt.Sprintf("INSERT INTO %s.%s (%s) VALUES (%s)",
 		schema, storageUnit, strings.Join(columns, ", "), strings.Join(placeholders, ", "))

-	err = conn.Exec(context.Background(), query, args...)
+	_, err = conn.ExecContext(context.Background(), query, args...)


To fix the problem, we need to use parameterized queries or prepared statements to safely embed user-controlled data into the SQL query. This approach prevents SQL injection by ensuring that user input is treated as data rather than executable code.

In the AddRow function, we should modify the query construction to use placeholders for the schema and storageUnit parameters and pass them as arguments to the ExecContext function. This will ensure that these parameters are safely embedded into the query.

core/src/plugins/clickhouse/clickhouse.go

@@ -66,7 +66,7 @@
 		WHERE database = '%s'
 	`, schema)

-	rows, err := conn.Query(context.Background(), query)
+	rows, err := conn.QueryContext(context.Background(), query)


To fix the problem, we need to use parameterized queries instead of string concatenation to safely include the user-provided schema parameter in the SQL query. This can be achieved by using the QueryContext method with placeholders for parameters.

Modify the GetStorageUnits function in core/src/plugins/clickhouse/clickhouse.go to use a parameterized query.

Replace the fmt.Sprintf usage with a query string that includes a placeholder for the schema parameter.

Pass the schema parameter as an argument to the QueryContext method.

core/src/plugins/clickhouse/clickhouse.go

@@ -112,7 +112,7 @@
 		ORDER BY position
 	`, schema, tableName)

-	rows, err := conn.Query(context.Background(), query)
+	rows, err := conn.QueryContext(context.Background(), query)


To fix the problem, we need to use parameterized queries instead of string concatenation to construct the SQL query. This will ensure that user input is properly escaped and cannot be used to inject malicious SQL code.

Replace the fmt.Sprintf usage with parameterized queries using ? placeholders.

Modify the getTableSchema function to accept the schema and tableName parameters as query arguments.

Update the conn.QueryContext call to pass the parameters separately.

core/src/plugins/clickhouse/delete.go

@@ -68,7 +73,7 @@
 		strings.Join(whereClauses, " AND "))

 	// Execute the query
-	err = conn.Exec(context.Background(), query, args...)
+	_, err = conn.ExecContext(context.Background(), query, args...)


To fix the problem, we need to ensure that user-provided data is safely embedded into the SQL query. This can be achieved by using parameterized queries or prepared statements. In this case, we will modify the code to use parameterized queries for the schema and storageUnit parameters.

Modify the DeleteRow function in core/src/plugins/clickhouse/delete.go to use parameterized queries for the schema and storageUnit parameters.

Update the query construction to include placeholders for the schema and storageUnit parameters.

Pass the schema and storageUnit parameters as arguments to the ExecContext function.

core/src/plugins/clickhouse/query.go

@@ -28,13 +28,16 @@
 	}
 	defer conn.Close()

-	rows, err := conn.Query(context.Background(), query, params)
+	rows, err := conn.QueryContext(context.Background(), query, params)


To fix the problem, we need to use parameterized queries instead of directly embedding user-controlled values into the SQL query string. This can be achieved by using placeholders (?) in the query string and passing the actual values as arguments to the QueryContext method.

Modify the GetRows function to use parameterized queries.

Update the executeQuery function to accept additional parameters for the query placeholders.

Ensure that the RawExecute function also uses parameterized queries if applicable.

core/src/plugins/clickhouse/update.go

@@ -99,7 +104,7 @@
 		strings.Join(setClauses, ", "),
 		strings.Join(whereClauses, " AND "))

-	err = conn.Exec(context.Background(), query, args...)
+	_, err = conn.ExecContext(context.Background(), query, args...)


To fix the problem, we need to ensure that the schema parameter is safely embedded into the SQL query. This can be achieved by using parameterized queries or prepared statements, which prevent SQL injection by treating user input as data rather than executable code.

In this specific case, we can use the sql.Named function to safely include the schema parameter in the query. This approach ensures that the schema value is properly escaped and treated as a parameter rather than part of the SQL command.

core/src/plugins/clickhouse/utils.go

@@ -18,7 +18,7 @@
 		WHERE database = '%s' AND table = '%s'`,
 		schema, table)

-	rows, err := conn.Query(context.Background(), query)
+	rows, err := conn.QueryContext(context.Background(), query)


To fix the problem, we need to use parameterized queries instead of string concatenation to construct the SQL query. This will ensure that user input is properly escaped and cannot be used to inject malicious SQL code.

Replace the fmt.Sprintf call with a parameterized query using placeholders.

Use the QueryContext method with the appropriate arguments to pass the user input as parameters to the query.

…ontend

… out of the params

…(http)

Update how databases connect to avoid parameter injection

Add Alias logic

Ensure ids are unique for columns with same name

…ckaged use

Replace monaco editor with code mirror

This reverts commit 46a7956.

apple and others added 20 commits December 2, 2024 21:19

add openai compatible provider

898d894

Merge branch 'clidey:main' into main

b12bc69

Merge branch 'main' into main

15fa9ca

add additional options for clickhouse support, HTTP protocol, TLS mod…

95026e8

…e, readonly and debug advanced options Signed-off-by: Slach <[email protected]>

Merge pull request #300 from Slach/main

1411fcf

add additional options for clickhouse support

Merge pull request #252 from learningpro/main

b994967

Add OpenAI Compatible Provider

fix variant A and B for path traversal

27f106b

remove string processing code

cb5b9bf

update directory to end with slash

cfda05a

initial fix for mysql to use the driver Config object to build the ds…

9c15d79

…n instead of string concat

change to hostPath check to account for length

cb871a0

removed the hostPath for now. removed & from collation before passing…

380544c

… to config

add back func for reading in params behind isProfile guard

9e8669a

postgres dsn builder surrounds all params with single quotes now to t…

903c223

…ake value as is

redis addr built using net.JoinHostPort

f2d6de1

modify mongodb to connect using clientOptions

dd94d97

query escape for elasticsearch

520d862

disable clickhouse sslmode as it's not fully complete. update elastic…

33a56d1

…search to build url using url.URL, further url.QueryEscapes

Update issue templates

3f6fcab

Merge commit from fork

547336a

fix for sqlite path traversal

github-advanced-security bot found potential problems Jan 27, 2025

View reviewed changes

modelorona changed the title ~~Release - Fix path traversal bug for sqlite and add several clickhouse options~~ Release - Fix path traversal bug for sqlite, new clickhouse options, and support for OpenAI-like URLs Jan 27, 2025

modelorona and others added 8 commits January 27, 2025 17:58

parse port as num and fail otherwise, remove collation option from fr…

0e75ff3

…ontend

update clickhouse config

6f1ebf3

update postgres handling to escape all single quotes to avoid jumping…

2d47a83

… out of the params

fix clickhouse setup to allow for both port 9000 (tcp) and port 8123 …

5978412

…(http)

Merge commit from fork

8d67b76

Update how databases connect to avoid parameter injection

feat(core,frontend): add alias logic

f67f843

Merge pull request #331 from clidey/hk/issues/alias

76e0cad

Add Alias logic

feat(frontend): fix table header logic to have unique ids

e2f0ed9

hkdeman added 8 commits January 28, 2025 11:47

feat(frontend): fix up undefined

114522f

Merge pull request #332 from clidey/hk/issues/scratchpad-wrong-columns

410561a

Ensure ids are unique for columns with same name

feat(frontend): replace monaco editor with code mirror for offline pa…

bef215f

…ckaged use

feat(editor): fix theming

df37d60

feat(frontend): fix markdown library issue

e7d8228

Merge pull request #333 from clidey/hk/issues/switch-to-code-mirror

c27f796

Replace monaco editor with code mirror

feat(frontend): add loader and loading states

46a7956

Revert "feat(frontend): add loader and loading states"

e19fe5a

This reverts commit 46a7956.

@@ -52,4 +52,4 @@
             	// Build the CREATE TABLE query
-            	query := fmt.Sprintf("CREATE TABLE %s.%s (\n\t%s\n) ENGINE = %s",
-            		schema, storageUnit, strings.Join(columns, ",\n\t"), engineSettings.engine)
+            	query := fmt.Sprintf("CREATE TABLE ?.? (\n\t%s\n) ENGINE = %s",
+            		strings.Join(columns, ",\n\t"), engineSettings.engine)
@@ -74,3 +74,3 @@
-            	_, err = conn.ExecContext(context.Background(), query)
+            	_, err = conn.ExecContext(context.Background(), query, sql.Named("schema", schema), sql.Named("storageUnit", storageUnit))
             	if err != nil {

@@ -58,3 +58,3 @@
-            	query := fmt.Sprintf(`
+            	query := `
             		SELECT
@@ -65,6 +65,6 @@
             		FROM system.tables
-            		WHERE database = '%s'
-            	`, schema)
+            		WHERE database = ?
+            	`
-            	rows, err := conn.QueryContext(context.Background(), query)
+            	rows, err := conn.QueryContext(context.Background(), query, schema)
             	if err != nil {

@@ -105,3 +105,3 @@
             func getTableSchema(conn *sql.DB, schema string, tableName string) ([]engine.Record, error) {
-            	query := fmt.Sprintf(`
+            	query := `
             		SELECT
@@ -110,7 +110,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 {

@@ -67,10 +67,8 @@
             	// Construct the DELETE query
-            	query := fmt.Sprintf(`
-            		ALTER TABLE %s.%s
-            		DELETE WHERE %s`,
-            		schema,
-            		storageUnit,
-            		strings.Join(whereClauses, " AND "))
+            	query := `
+            		ALTER TABLE ?.?
+            		DELETE WHERE ` + strings.Join(whereClauses, " AND ")
             	// Execute the query
+            	args = append([]interface{}{schema, storageUnit}, args...)
             	_, err = conn.ExecContext(context.Background(), query, args...)

@@ -10,9 +10,6 @@
             func (p *ClickHousePlugin) GetRows(config *engine.PluginConfig, schema string, storageUnit string, where string, pageSize int, pageOffset int) (*engine.GetRowsResult, error) {
-            	query := fmt.Sprintf("SELECT * FROM %s.%s", schema, storageUnit)
-            	if where != "" {
-            		query += " WHERE " + where
-            	}
-            	query += fmt.Sprintf(" LIMIT %d OFFSET %d", pageSize, pageOffset)
+            	query := "SELECT * FROM ?.? WHERE ? LIMIT ? OFFSET ?"
+            	params := []interface{}{schema, storageUnit, where, pageSize, pageOffset}
-            	return p.executeQuery(config, query)
+            	return p.executeQuery(config, query, params...)
             }
@@ -30,3 +27,3 @@
-            	rows, err := conn.QueryContext(context.Background(), query, params)
+            	rows, err := conn.QueryContext(context.Background(), query, params...)
             	if err != nil {

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release - Fix path traversal bug for sqlite, new clickhouse options, and support for OpenAI-like URLs #329

Release - Fix path traversal bug for sqlite, new clickhouse options, and support for OpenAI-like URLs #329

modelorona commented Jan 27, 2025 •

edited

Loading

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Release - Fix path traversal bug for sqlite, new clickhouse options, and support for OpenAI-like URLs #329

Are you sure you want to change the base?

Release - Fix path traversal bug for sqlite, new clickhouse options, and support for OpenAI-like URLs #329

Conversation

modelorona commented Jan 27, 2025 • edited Loading

✨ Improvements

🐛 Bug Fixes

modelorona commented Jan 27, 2025 •

edited

Loading