Skip to content

Commit 93f5ef0

Browse files
fix: sanitize credentials from connection string parsing errors (#319)
Prevent leaking usernames and passwords in error messages when URL parsing fails. The original url.Parse error could include the full connection string with credentials. - Replace url.Parse error with generic message - Add tests using testify to verify credentials are not leaked Co-authored-by: David Levy <dlevy@microsoft.com>
1 parent fdee61d commit 93f5ef0

2 files changed

Lines changed: 39 additions & 2 deletions

File tree

msdsn/conn_str.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ func Parse(dsn string) (Config, error) {
647647
if !ok {
648648
epaString = os.Getenv("MSSQL_USE_EPA")
649649
}
650-
if epaString != "" {
650+
if epaString != "" {
651651
epaEnabled, err := strconv.ParseBool(epaString)
652652
if err != nil {
653653
return p, fmt.Errorf("invalid epa enabled value '%s': %v", epaString, err)
@@ -836,7 +836,8 @@ func splitConnectionStringURL(dsn string) (map[string]string, error) {
836836

837837
u, err := url.Parse(dsn)
838838
if err != nil {
839-
return res, err
839+
// Do not include the original error which may contain credentials
840+
return res, fmt.Errorf("unable to parse connection string: invalid URL format")
840841
}
841842

842843
if u.Scheme != "sqlserver" {

msdsn/conn_str_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,42 @@ func TestInvalidConnectionString(t *testing.T) {
5353
}
5454
}
5555

56+
func TestCredentialNotLeakedInError(t *testing.T) {
57+
// Test that when url.Parse fails, the error message does not contain credentials
58+
testCases := []struct {
59+
name string
60+
connStr string
61+
username string
62+
password string
63+
}{
64+
{
65+
name: "URL with invalid control character",
66+
connStr: "sqlserver://myuser:secretpassword@host:1433\x00invalid",
67+
username: "myuser",
68+
password: "secretpassword",
69+
},
70+
{
71+
name: "URL with password and null byte in query",
72+
connStr: "sqlserver://admin:mysecret123@server.example.com:1433?database=test\x00",
73+
username: "admin",
74+
password: "mysecret123",
75+
},
76+
}
77+
78+
for _, tc := range testCases {
79+
t.Run(tc.name, func(t *testing.T) {
80+
_, err := Parse(tc.connStr)
81+
if !assert.Error(t, err, "Expected error for invalid connection string") {
82+
return
83+
}
84+
85+
errMsg := err.Error()
86+
assert.NotContains(t, errMsg, tc.password, "Error message should not contain password")
87+
assert.NotContains(t, errMsg, tc.username, "Error message should not contain username")
88+
})
89+
}
90+
}
91+
5692
func TestValidConnectionString(t *testing.T) {
5793
type testStruct struct {
5894
connStr string

0 commit comments

Comments
 (0)