fix(js): escape dbname in lib/pq URL (#7479)

`PGClient.ExecuteQuery` built the connection URL
by interpolating the dbname directly into the path.
A value containing '?' could start the query string
and inject lib/pq options such as `sslrootcert`
before the appended `sslmode=disable`.

Build the URL from escaped userinfo, path and query
values so dbname stays part of the dbname.

Signed-off-by: Dwi Siswanto <[email protected]>

pkg/js/libs/postgres/postgres.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"net/url"
 	"strings"
 	"time"
 
@@ -125,7 +126,7 @@ func executeQuery(ctx context.Context, executionId string, host string, port int
 
 	target := net.JoinHostPort(host, fmt.Sprintf("%d", port))
 
-	connStr := fmt.Sprintf("postgres://%s:%s@%s/%s?sslmode=disable&executionId=%s", username, password, target, dbName, executionId)
+	connStr := buildPostgresConnURL(username, password, target, dbName, executionId)
 	db, err := pgwrap.OpenDB(ctx, executionId, connStr)
 	if err != nil {
 		return nil, err
@@ -145,6 +146,19 @@ func executeQuery(ctx context.Context, executionId string, host string, port int
 	return resp, nil
 }
 
+func buildPostgresConnURL(username, password, target, dbName, executionId string) string {
+	values := url.Values{}
+	values.Set("sslmode", "disable")
+	values.Set("executionId", executionId)
+
+	return fmt.Sprintf("postgres://%s@%s/%s?%s",
+		url.UserPassword(username, password).String(),
+		target,
+		url.PathEscape(dbName),
+		values.Encode(),
+	)
+}
+
 // ConnectWithDB connects to Postgres database using given credentials and database name.
 // If connection is successful, it returns true.
 // If connection is unsuccessful, it returns false and error.

pkg/js/libs/postgres/postgres_test.go

@@ -0,0 +1,74 @@
+package postgres
+
+import (
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/lib/pq"
+)
+
+func TestBuildPostgresConnectionURLDoesNotAllowDBNameQueryInjection(t *testing.T) {
+	dbName := "testdb?sslrootcert=/etc/passwd&sslmode=verify-ca&junk="
+	connStr := buildPostgresConnURL("postgres", "x", "127.0.0.1:5432", dbName, "exec-1")
+
+	u, err := url.Parse(connStr)
+	if err != nil {
+		t.Fatalf("parse connection URL: %v", err)
+	}
+
+	if got := strings.TrimPrefix(u.Path, "/"); got != dbName {
+		t.Fatalf("database name = %q, want %q", got, dbName)
+	}
+
+	values := u.Query()
+	if got := values.Get("sslmode"); got != "disable" {
+		t.Fatalf("sslmode = %q, want disable", got)
+	}
+	if got := values.Get("executionId"); got != "exec-1" {
+		t.Fatalf("executionId = %q, want exec-1", got)
+	}
+	deniedParams := []string{"sslrootcert", "sslcert", "sslkey", "service", "junk"}
+	for _, denied := range deniedParams {
+		if got := values.Get(denied); got != "" {
+			t.Fatalf("%s was injected with value %q", denied, got)
+		}
+	}
+
+	pqDSN, err := pq.ParseURL(connStr) //nolint:staticcheck // validates lib/pq URL parsing of the generated DSN.
+	if err != nil {
+		t.Fatalf("parse connection URL as lib/pq DSN: %v", err)
+	}
+	if !strings.Contains(pqDSN, "dbname='"+dbName+"'") {
+		t.Fatalf("lib/pq DSN = %q, want dbName preserved as dbname", pqDSN)
+	}
+	for _, denied := range deniedParams {
+		if strings.Contains(pqDSN, " "+denied+"=") {
+			t.Fatalf("%s was injected into lib/pq DSN %q", denied, pqDSN)
+		}
+	}
+}
+
+func TestBuildPostgresConnectionURLEscapesCredentials(t *testing.T) {
+	username := "user:[email protected]"
+	password := "pa:ss@word?x"
+	connStr := buildPostgresConnURL(username, password, "127.0.0.1:5432", "postgres", "exec-1")
+
+	u, err := url.Parse(connStr)
+	if err != nil {
+		t.Fatalf("parse connection URL: %v", err)
+	}
+
+	if got := u.User.Username(); got != username {
+		t.Fatalf("username = %q, want %q", got, username)
+	}
+	if got, _ := u.User.Password(); got != password {
+		t.Fatalf("password = %q, want %q", got, password)
+	}
+	if got := u.Host; got != "127.0.0.1:5432" {
+		t.Fatalf("host = %q, want 127.0.0.1:5432", got)
+	}
+	if got := strings.TrimPrefix(u.Path, "/"); got != "postgres" {
+		t.Fatalf("database name = %q, want postgres", got)
+	}
+}