Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

DataObjects logs database password on database error #18

Closed
rsutphin opened this issue Dec 28, 2011 · 5 comments
Closed

DataObjects logs database password on database error #18

rsutphin opened this issue Dec 28, 2011 · 5 comments
Assignees

Comments

@rsutphin
Copy link

When there's a database exception thrown DO includes the database password in the logs. Example:

 DM: ERROR:  duplicate key value violates unique constraint "tsu_pkey"
DETAIL:  Key (tsu_id)=(.) already exists. (code: 83906754, sql state: 23505, query: INSERT INTO "tsu" ("sc_id", "psu_id", "tsu_id", "tsu_name") VALUES ('200000XX', '200000XX', '.', '.'), uri: postgres:mdes_warehouse:ACTUAL_PASSWORD@server:5432mdes_warehouse_working?username=mdes_warehouse&database=mdes_warehouse_working&adapter=postgres&host=server&port=5432&password=ACTUAL_PASSWORD)

I'm seeing this with DO 0.10.7 and do_postgres 0.10.7. I'm using DO within DataMapper 1.2.0. I mentioned this issue on the datamapper mailing list and it was suggested that it's a DO bug.

@ghost ghost assigned myabc Dec 28, 2011
@myabc
Copy link
Member

myabc commented Dec 28, 2011

@dbussink @dkubb Are you in agreement we should elide this information from our logging?

@dbussink
Copy link
Contributor

Yeah, we should not output this information in exceptions etc. so we should fix this behavior.

@mkristian
Copy link
Member

definitely this info should stay away from log files !

@dkubb
Copy link
Member

dkubb commented Dec 29, 2011

@myabc I agree, we definitely should not display this information in exception output.

The question is, where should the change be made and who should make it? @dbussink and @myabc, are you able to change the C and Java drivers respectively, or would someone else be better for making these changes?

@systemed
Copy link

systemed commented Jan 29, 2018

This only half-fixes the issue. In @rsutphin's original report, uri contained:

postgres:mdes_warehouse:ACTUAL_PASSWORD@server:5432mdes_warehouse_working?username=mdes_warehouse&database=mdes_warehouse_working&adapter=postgres&host=server&port=5432&password=ACTUAL_PASSWORD

The first occurrence of ACTUAL_PASSWORD is now removed, but the second one is still present, serialised by iterating through the query attribute.

This can be fixed by changing the relevant line to

string << "?" << query.select {|k,v| k!='password' }.map do |key, value|

(I know this is seriously old code, but logging passwords is a seriously big bug. ;) )

systemed added a commit to systemed/do that referenced this issue Jan 29, 2018
Currently passwords are logged if they're part of `query` - see datamapper#18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants