CVE-2023-30625: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: rudder-server is part of RudderStack, an open source Customer Data Platform (CDP). Versions of rudder-server prior to 1.3.0-rc.1 are vulnerable to SQL injection. This issue may lead to Remote Code Execution (RCE) due to the `rudder` role in PostgresSQL having superuser permissions by default. Version 1.3.0-rc.1 contains patches for this issue.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 8.8
Published: 2023-06-16T17:15Z
Repository: https://github.com/rudderlabs/rudder-server
Language: Go
Files changed: router/failed-events-manager.go
Commit date: 2022-11-07 21:10:24 +0200
Commit message: fix: always use a sql safe table name in failed events manager (#2664)


Diff: diff --git a/router/failed-events-manager.go b/router/failed-events-manager.go
index b76908fb..a91d6ea1 100644
--- a/router/failed-events-manager.go
+++ b/router/failed-events-manager.go
@@ -54,11 +54,11 @@ func (*FailedEventsManagerT) SaveFailedRecordIDs(taskRunIDFailedEventsMap map[st
 	if !failedKeysEnabled {
 		return
 	}
 
 	for taskRunID, failedEvents := range taskRunIDFailedEventsMap {
-		table := `"` + strings.ReplaceAll(fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID), `"`, `""`) + `"`
+		table := getSqlSafeTablename(taskRunID)
 		sqlStatement := fmt.Sprintf(`CREATE TABLE IF NOT EXISTS %s (
 		destination_id TEXT NOT NULL,
 		record_id JSONB NOT NULL,
 		created_at TIMESTAMP NOT NULL);`, table)
 		_, err := txn.Exec(sqlStatement)
@@ -92,11 +92,11 @@ func (fem *FailedEventsManagerT) DropFailedRecordIDs(taskRunID string) {
 	if !failedKeysEnabled {
 		return
 	}
 
 	// Drop table
-	table := fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID)
+	table := getSqlSafeTablename(taskRunID)
 	sqlStatement := fmt.Sprintf(`DROP TABLE IF EXISTS %s`, table)
 	_, err := fem.dbHandle.Exec(sqlStatement)
 	if err != nil {
 		pkgLogger.Errorf("Failed to drop table %s with error: %v", taskRunID, err)
 	}
@@ -109,11 +109,11 @@ func (fem *FailedEventsManagerT) FetchFailedRecordIDs(taskRunID string) []*Faile
 
 	failedEvents := make([]*FailedEventRowT, 0)
 
 	var rows *sql.Rows
 	var err error
-	table := `"` + strings.ReplaceAll(fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID), `"`, `""`) + `"`
+	table := getSqlSafeTablename(taskRunID)
 	sqlStatement := fmt.Sprintf(`SELECT %[1]s.destination_id, %[1]s.record_id
                                              FROM %[1]s `, table)
 	rows, err = fem.dbHandle.Query(sqlStatement)
 	if err != nil {
 		pkgLogger.Errorf("Failed to fetch from table %s with error: %v", taskRunID, err)
@@ -186,5 +186,9 @@ func CleanFailedRecordsTableProcess(ctx context.Context) {
 }
 
 func (fem *FailedEventsManagerT) GetDBHandle() *sql.DB {
 	return fem.dbHandle
 }
+
+func getSqlSafeTablename(taskRunID string) string {
+	return `"` + strings.ReplaceAll(fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID), `"`, `""`) + `"`
+}

Vulnerable code: 		table := `"` + strings.ReplaceAll(fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID), `"`, `""`) + `"`
	table := fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID)
	table := `"` + strings.ReplaceAll(fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID), `"`, `""`) + `"`
Fixed code: 		table := getSqlSafeTablename(taskRunID)
	table := getSqlSafeTablename(taskRunID)
	table := getSqlSafeTablename(taskRunID)

func getSqlSafeTablename(taskRunID string) string {
	return `"` + strings.ReplaceAll(fmt.Sprintf(`%s_%s`, failedKeysTablePrefix, taskRunID), `"`, `""`) + `"`
}