Skip to content

Commit 4f53f78

Browse files
committed
external connection: fix sql injection vuln in ALTER EXTERNAL CONNECTION
Previously, the sql command which ran directly on system.external_connections passed the external connection name via string parsing, which makes the query vulnerable to sql injection. This patch fixes this vulnerability by passing the name as a parameter. Epic: none Release note: none
1 parent 191e4a1 commit 4f53f78

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

pkg/cloud/externalconn/record.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,20 +355,26 @@ func (e *MutableExternalConnection) Create(ctx context.Context, txn isql.Txn) er
355355
}
356356

357357
func (e *MutableExternalConnection) Update(ctx context.Context, txn isql.Txn) error {
358-
cols, qargs, err := e.marshalChanges()
358+
cols, qargsForChanges, err := e.marshalChanges()
359359
if err != nil {
360360
return err
361361
}
362362

363+
qargs := make([]interface{}, 0, 1+len(qargsForChanges))
364+
qargs = append(qargs, e.ConnectionName())
365+
qargs = append(qargs, qargsForChanges...)
366+
363367
var setClauses []string
364-
for i, col := range cols {
365-
setClauses = append(setClauses, fmt.Sprintf("%s = $%d", col, i+1))
368+
placeHolder := 2
369+
for _, col := range cols {
370+
setClauses = append(setClauses, fmt.Sprintf("%s = $%d", col, placeHolder))
371+
placeHolder++
366372
}
367373

368374
updateQuery := fmt.Sprintf(`UPDATE system.external_connections
369375
SET %s, updated = now()
370-
WHERE connection_name = '%s'`,
371-
strings.Join(setClauses, ", "), e.ConnectionName())
376+
WHERE connection_name = $1`,
377+
strings.Join(setClauses, ", "))
372378

373379
_, err = txn.ExecEx(ctx, "ExternalConnection.Update", txn.KV(), sessiondata.NodeUserSessionDataOverride, updateQuery, qargs...)
374380
return err

0 commit comments

Comments
 (0)