Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security issue: password leaking to logs #240

Open
ctholho opened this issue Feb 2, 2024 · 1 comment
Open

Security issue: password leaking to logs #240

ctholho opened this issue Feb 2, 2024 · 1 comment
Assignees

Comments

@ctholho
Copy link

ctholho commented Feb 2, 2024

GORM Playground Link

n/a

Description

When gorm fails to connect to e.g. postgres the resulting stack trace logs the *pgconn.Config Object, including the database password, to the logs. I think that's a security issue that should be fixed on the level of gorm.

We have error logs like this

{"level":"fatal","ts":1706803743.2407858,"caller":"sqlDatabase/database.go:37","msg":"failed to connect database: &{%!e(*pgconn.Config=&{postgres.svc.cluster.local 5432 user_name ⚠️password⚠️ 0xc0000a3520 0 0x751c80 0x88af40 0x889a60 map[]   [0xc000584f60] <nil> <nil> <nil> 0x9ae720 true}) %!e(string=hostname resolving error) %!e(*net.DNSError=&{no such host postgres.svc.cluster.local 10.43.0.10:53 false false true})}","stacktrace":"app/src/bla/bla/bla/database.go:37\nsync.(*Once).doSlow\n\t/usr/local/go/src/sync/once.go:74\nsync.(*Once).Do\n\t/usr/local/go/src/sync/once.go:65\blablabla/sqlDatabase.GetDatabaseInstance\n\t/app/src/blablabla/database.go:25\nmain.main\n\t/app/src/main.go:56\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:267"}

We try to open a connection like this:

db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{})
if err != nil {
	logger.Fatalf("failed to connect database: %e", err)
}

Proposed fix:
On an application level, it's generally good to know why database connections failed, so we want to log the error. But relying on users to properly filter out the password in pgconn.Config is big burden. Because password leaking through logs is a known problem, I think this should be considered a serious issue and not merely a feature request.

I think a big decision to be made is if this should be handled centrally by the gorm lib or for each connector separately.

@garrettladley
Copy link

IMO should be handled at the gorm level instead of on a connector by connector level. Opened this issue in gorm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants