diff --git a/pkg/database/database.go b/pkg/database/database.go index 0c540075..f090a777 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -533,7 +533,11 @@ func nativeDB2DSN(opts ConnectOptions) (string, string, bool, error) { if err != nil { return "", "", false, err } - if _, native := db2.ParseNativeDSN(dsn); !native { + _, native, err := db2.ParseNativeDSN(dsn) + if err != nil { + return "", "", false, fmt.Errorf("invalid native DB2 DSN: %w", err) + } + if !native { return "", "", false, nil } // Confirmed native: re-expand with keyword-injection validation. The expansion above @@ -542,7 +546,10 @@ func nativeDB2DSN(opts ConnectOptions) (string, string, bool, error) { if err != nil { return "", "", false, err } - database, _ := db2.ParseNativeDSN(safeDSN) + database, _, err := db2.ParseNativeDSN(safeDSN) + if err != nil { + return "", "", false, fmt.Errorf("invalid native DB2 DSN: %w", err) + } return safeDSN, database, true, nil } diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 278abc7e..7a9f27c4 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -1,6 +1,7 @@ package db2 import ( + "errors" "fmt" "net/url" "regexp" @@ -8,22 +9,27 @@ import ( "strings" ) -// urlSchemeRegex matches a DSN that begins with a URL scheme (e.g. "db2://"); anchoring -// to the start keeps a native DSN whose value contains "://" (e.g. PWD=my://secret) from -// being misread as a URL. +// urlSchemeRegex matches a DSN that begins with a URL scheme, so a native DSN whose value +// contains "://" isn't misread as a URL. var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) +// ErrAmbiguousDSN means a brace-quoted value appears to contain a later KEYWORD=value field. +// The DSN is rejected rather than guessed at, since it may carry credentials. +var ErrAmbiguousDSN = errors.New("ambiguous DB2 DSN: a brace-quoted value appears to contain a later field") + // ParseNativeDSN reports whether dsn is DB2's native ODBC keyword=value form (not a URL), -// returning its DATABASE value if present. HOSTNAME, not DATABASE alone, is the native -// marker, since other engines' ODBC/ADO strings also carry DATABASE; every caller shares -// this one detector to avoid drift. -func ParseNativeDSN(dsn string) (string, bool) { +// returning its DATABASE value if present. HOSTNAME, not DATABASE alone, is the native marker. +func ParseNativeDSN(dsn string) (string, bool, error) { if urlSchemeRegex.MatchString(dsn) { - return "", false + return "", false, nil + } + parts, err := splitDB2DSN(dsn) + if err != nil { + return "", false, err } var database string native, haveDB := false, false - for _, part := range splitDB2DSN(dsn) { + for _, part := range parts { keyword, value, found := strings.Cut(part, "=") if !found { continue @@ -42,56 +48,119 @@ func ParseNativeDSN(dsn string) (string, bool) { } } } - return database, native + return database, native, nil } -// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form. +// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form. An ambiguous DSN +// is treated as not native. func IsNativeDSN(dsn string) bool { - _, native := ParseNativeDSN(dsn) - return native + _, native, err := ParseNativeDSN(dsn) + return err == nil && native } -// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent. +// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent or ambiguous. func DSNDatabase(dsn string) string { - database, _ := ParseNativeDSN(dsn) + database, _, _ := ParseNativeDSN(dsn) return database } -// splitDB2DSN splits a native DB2 DSN on ';', treating '{' as ODBC quoting only when it -// opens a value and is later closed by '}'; an unterminated or misplaced '{' is literal, -// so HOSTNAME/DATABASE markers stay visible instead of being silently swallowed. -func splitDB2DSN(dsn string) []string { +// matchBraces pairs each reserved-keyword's opening '{' with the '}' that closes it via LIFO +// stack matching, so an earlier unterminated '{' can't steal a later value's closing '}'. Only +// a '{' right after a reserved keyword's '=' is pushed, so a '=' occurring inside an +// already-open value (ODBC values don't nest) can't be mistaken for a new field's opener. +// Ambiguous braces have no entry in the returned map. +func matchBraces(s string) map[int]int { + pairs := make(map[int]int) + var stack []int + wordStart, eqPos := 0, -1 + atValueStart := false + for i := 0; i < len(s); i++ { + switch s[i] { + case '{': + if atValueStart && eqPos >= 0 && reservedDSNKeywords[strings.ToUpper(strings.TrimSpace(s[wordStart:eqPos]))] { + stack = append(stack, i) + } + atValueStart = false + case '}': + if n := len(stack); n > 0 { + open := stack[n-1] + stack = stack[:n-1] + pairs[open] = i + } + atValueStart = false + case '=': + eqPos = i + atValueStart = true + case ';': + wordStart = i + 1 + atValueStart = false + case ' ', '\t': + // keep atValueStart across whitespace before a brace. + default: + atValueStart = false + } + } + return pairs +} + +// bareFieldPattern captures a "KEYWORD=" immediately after a ';' inside a brace-quoted span. +var bareFieldPattern = regexp.MustCompile(`;\s*([A-Za-z][A-Za-z0-9_]*)=`) + +// swallowedReservedField reports whether span contains what looks like a later reserved +// KEYWORD=value field, and returns that keyword. Only reserved keywords count, so a value +// that merely contains ";word=" text isn't misclassified as ambiguous. +func swallowedReservedField(span string) (string, bool) { + for _, m := range bareFieldPattern.FindAllStringSubmatch(span, -1) { + if keyword := strings.ToUpper(m[1]); reservedDSNKeywords[keyword] { + return keyword, true + } + } + return "", false +} + +// splitDB2DSN splits a native DB2 DSN on ';', treating '{' as ODBC quoting only when it opens +// a value with a genuine matching '}'. A span that appears to swallow a later reserved field +// makes the DSN ambiguous; the resulting error reports only keyword names, never values. +func splitDB2DSN(dsn string) ([]string, error) { + pairs := matchBraces(dsn) var parts []string start := 0 - braced := false // inside a {...} quoted value + braceEnd := -1 // index of the '}' that closes the current quoted value, or -1 atValueStart := false // at a value position (right after '=', across whitespace) outside braces for i := 0; i < len(dsn); i++ { + if braceEnd != -1 { + if i == braceEnd { + braceEnd = -1 + atValueStart = false + } + continue + } switch dsn[i] { - case '}': - braced = false - atValueStart = false case '{': - if atValueStart && strings.IndexByte(dsn[i:], '}') != -1 { - braced = true + if atValueStart { + if end, ok := pairs[i]; ok { + if swallowed, ambiguous := swallowedReservedField(dsn[i+1 : end]); ambiguous { + owner, _, _ := strings.Cut(dsn[start:i], "=") + return nil, fmt.Errorf("%w (keyword %q appears to swallow a later %q field)", + ErrAmbiguousDSN, strings.TrimSpace(owner), swallowed) + } + braceEnd = end + } } atValueStart = false case '=': - if !braced { - atValueStart = true - } + atValueStart = true case ';': - if !braced { - parts = append(parts, dsn[start:i]) - start = i + 1 - } + parts = append(parts, dsn[start:i]) + start = i + 1 atValueStart = false case ' ', '\t': - // keep atValueStart so "DATABASE= {my;db}" still brace-detects. + // keep atValueStart across whitespace before a brace. default: atValueStart = false } } - return append(parts, dsn[start:]) + return append(parts, dsn[start:]), nil } // Keywords derived from the URL itself; query parameters may not override them. @@ -122,7 +191,11 @@ func quoteDB2Value(v string) (string, error) { func convertToDB2DSN(dsn string) (string, error) { // If it's already in DB2's native keyword=value format, return as-is. // URL-format DSNs are exempt so those markers may appear in credentials. - if IsNativeDSN(dsn) { + _, native, err := ParseNativeDSN(dsn) + if err != nil { + return "", fmt.Errorf("invalid native DB2 DSN: %w", err) + } + if native { return dsn, nil } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index a3c0b776..c79f1217 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -128,12 +128,16 @@ func TestIsNativeDSN(t *testing.T) { {name: "space before the =", dsn: "HOSTNAME = h;DATABASE=X", want: true}, // DATABASE without HOSTNAME is a generic ODBC/ADO shape (e.g. MSSQL), not native DB2. {name: "database without hostname is not native", dsn: "Server=x;Database=y;User Id=u", want: false}, - // HOSTNAME appears only inside a braced PWD value, so the brace-aware split keeps it - // as one PWD part: not a native marker. + // A reserved keyword found after a ';' inside a braced value is rejected as ambiguous. {name: "hostname marker only inside braced value", dsn: "UID=u;PWD={x;HOSTNAME=y}", want: false}, + // A reserved keyword with no leading ';' can't look like a separate field, so it's kept + // buried without erroring. + {name: "hostname marker directly inside braced value does not error", dsn: "UID=u;PWD={HOSTNAME=y}", want: false}, // Unterminated '{' is literal, so the ';' still splits and HOSTNAME= stays visible; // the malformed value then reaches the driver instead of silently misrouting. {name: "unterminated brace keeps marker visible", dsn: "PWD={oops;HOSTNAME=h", want: true}, + // The later '{' pairs with DATABASE's '}', not PWD's, so HOSTNAME stays visible. + {name: "hostname visible despite later legitimately-braced value", dsn: "HOSTNAME=h;PWD={oops;DATABASE={REAL}", want: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -160,6 +164,21 @@ func TestDSNDatabase(t *testing.T) { // A literal '{' mid-value (not ODBC quoting) must not swallow the following ';'. {name: "unquoted brace in earlier value", dsn: "HOSTNAME=h;PWD=p{q;DATABASE=TESTDB", want: "TESTDB"}, {name: "absent", dsn: "HOSTNAME=h;UID=u", want: ""}, + // An earlier unterminated '{' must not steal DATABASE's closing '}'. + {name: "unterminated brace does not steal a later value's closing brace", dsn: "HOSTNAME=h;PWD={oops;DATABASE={REAL}", want: "REAL"}, + {name: "two unterminated braces before the real one", dsn: "HOSTNAME=h;A={one;B={two;DATABASE={REAL}", want: "REAL"}, + {name: "three unterminated braces before the real one", dsn: "HOSTNAME=h;A={one;B={two;C={three;DATABASE={REAL}", want: "REAL"}, + // A later unterminated brace must not retroactively corrupt an earlier, already-closed value. + {name: "real value first, unterminated brace after", dsn: "HOSTNAME=h;DATABASE={REAL};PWD={oops", want: "REAL"}, + // A stray '}' with no preceding '{' has nothing to pair with and stays literal. + {name: "stray closing brace with no opener", dsn: "HOSTNAME=h;DATABASE=TESTDB};UID=u", want: "TESTDB}"}, + // Only a reserved keyword after the ';' makes a braced value ambiguous. + {name: "braced password with semicolon and non-reserved word is not ambiguous", dsn: "HOSTNAME=h;DATABASE=db;UID=u;PWD={pa;ss=word}", want: "db"}, + // A literal '{' inside a braced value isn't a new opener, since ODBC values don't nest. + {name: "literal brace inside a braced value does not truncate it", dsn: "HOSTNAME=h;DATABASE={a{b;c};UID=u", want: "a{b;c"}, + // A non-reserved keyword's '=' inside an already-open value must not open a new + // candidate brace, or it steals the real closing brace and truncates the value. + {name: "non-reserved keyword inside a braced value does not steal its closing brace", dsn: "HOSTNAME=h;DATABASE={db;x={y};UID=u", want: "db;x={y"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -167,3 +186,24 @@ func TestDSNDatabase(t *testing.T) { }) } } + +func TestParseNativeDSN_Ambiguous(t *testing.T) { + tests := []struct { + name string + dsn string + }{ + // An earlier unterminated '{' pairs with a bare field's stray trailing '}' instead of + // its own value, swallowing that field. + {name: "unterminated brace swallows a later bare field", dsn: "HOSTNAME=h;PWD={oops;DATABASE=TESTDB}"}, + {name: "unterminated brace swallows multiple later bare fields", dsn: "HOSTNAME=h;PWD={oops;UID=u;DATABASE=TESTDB}"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := ParseNativeDSN(tt.dsn) + require.ErrorIs(t, err, ErrAmbiguousDSN) + // The error must never echo any part of the DSN's values, which may be credentials. + require.NotContains(t, err.Error(), "oops") + require.NotContains(t, err.Error(), "TESTDB") + }) + } +} diff --git a/pkg/database/native_db2_dsn_test.go b/pkg/database/native_db2_dsn_test.go index b5afda9c..a13261ac 100644 --- a/pkg/database/native_db2_dsn_test.go +++ b/pkg/database/native_db2_dsn_test.go @@ -68,6 +68,11 @@ func TestNativeDB2DSN(t *testing.T) { opts: ConnectOptions{DSN: "HOSTNAME=${MISSING};DATABASE=d", Lookup: lookup(map[string]string{})}, wantErr: "MISSING", }, + { + name: "ambiguous brace swallowing a later field errors", + opts: ConnectOptions{DSN: "HOSTNAME=h;PWD={oops;DATABASE=TESTDB}"}, + wantErr: "ambiguous DB2 DSN", + }, } for _, tt := range tests {