go-gorm / postgres

GORM PostgreSQL driver
MIT License
225 stars 119 forks source link

Security issue: password leaking to logs #240

Closed ctholho closed 2 months ago

ctholho commented 6 months ago

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 commented 6 months ago

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

jinzhu commented 2 months ago

We do not log any information. The error mentioned above comes from the driver, so it should be addressed and fixed within the driver itself.

jinzhu commented 2 months ago

the driver means jackc/pgx