| Subject: | Username and Password quoting bug in dbdimp.c |
Hello,
I was testing out some code for escaping Postgres connection strings
today, and found that DBD::Pg doesn't quote usernames or passwords
properly when creating a connection string. This makes it impossible
to connect to a database with a username or password containing a
single quote or a backslash. It also seems to open up a security hole
if a username or password are under user control.
In the process of testing, I created a user with a password of:
\scott's"place"\
and found that DBD::Pg couldn't connect as that user. Some
investigation showed that in dbdimp.c, usernames and passwords are not
escaped at all.
In an application where the database username or password are under
user control (this is true in my application, where we take a Postgres
username and password from a Web form to authenticate and later
execute SQL statements), this lets a user set any database parameter.
For example, look at the connection string that's used if I enter a
password of:
s3cr3t host=127.0.0.1 port=6543
env DBI_TRACE=9 perl -Iblib/lib -Iblib/arch -MDBI -e'
$dbh = DBI->connect("DBI:Pg:dbname=vahrs",
"sgifford",
"s3cr3t host=127.0.0.1 port=6543")
or die "connect error: $DBI::errstr\n"
' 2>&1 |grep conn_str
pg_db_login: conn_str = >dbname=vahrs user=sgifford password=s3c3rt host=127.0.0.1 port=6\
543<
In this case, a user could cause authentication to happen against a
database of their choosing, which would let them access our
application without a valid username and password.
The attached patch escapes single quotes and backslashes with a
backslash; this works, and seems to be The Right Thing to Do according
to:
http://www.postgresql.org/docs/7.2/static/libpq-connect.html
diff -u DBD-Pg-1.32/dbdimp.c DBD-Pg-1.32-sg/dbdimp.c
--- DBD-Pg-1.32/dbdimp.c Tue Feb 3 14:50:22 2004
+++ DBD-Pg-1.32-sg/dbdimp.c Thu Jan 27 02:43:18 2005
@@ -36,6 +36,25 @@
#include "large_object.c"
#include "prescan_stmt.c"
+char *
+_strcat_esc(dest,s)
+ char *dest, *s;
+{
+ char *d = dest;
+
+ while (*d)
+ d++;
+
+ while (*s) {
+ if (*s == '\'' || *s == '\\')
+ *(d++)='\\';
+ *(d++)=*(s++);
+ }
+ *d='\0';
+
+ return dest;
+}
+
void
dbd_init (dbistate)
dbistate_t *dbistate;
@@ -142,7 +161,7 @@
/* DBD-Pg syntax: 'dbname=dbname;host=host;port=port' */
/* pgsql syntax: 'dbname=dbname host=host port=port user=uid password=pwd' */
- conn_str = (char *)safemalloc(strlen(dbname) + strlen(uid) + strlen(pwd) + 16 + 1);
+ conn_str = (char *)safemalloc(strlen(dbname) + 2*strlen(uid) + 2*strlen(pwd) + 16 + 4 + 1);
if (! conn_str)
return 0;
@@ -162,11 +181,13 @@
*dest = '\0';
if (strlen(uid)) {
- strcat(conn_str, " user=");
- strcat(conn_str, uid);
+ strcat(conn_str, " user='");
+ _strcat_esc(conn_str,uid);
+ strcat(conn_str,"'");
if (strlen(pwd)) {
- strcat(conn_str, " password=");
- strcat(conn_str, pwd);
+ strcat(conn_str, " password='");
+ _strcat_esc(conn_str, pwd);
+ strcat(conn_str,"'");
}
}