Package: libpam-pgsql Version: 0.5.2-9+b1 Severity: normal Tags: patch Hi,
the current libpam-pgsql library escapes the SQL options table name and various column names specified from within the pam configuration file or the pam_pgsql.conf. This prevents more complex SQL constructs within those parameters (such as nested SQL statements instead of a simple table or view name) without providing any real additional security, as the options' source should be trustworthy anyway (only system administrators can modify the PAM configuration or pam_pgsql.conf files). Nothing was changed regarding user provided input (such as the user name). The attached patch removes the unnecessary escaping and is tested on our systems. Please apply. Greetings, Fabian -- System Information: Debian Release: 4.0 APT prefers stable APT policy: (800, 'stable'), (3, 'testing'), (2, 'unstable'), (1, 'experimental') Architecture: i386 (i686) Shell: /bin/sh linked to /bin/bash Kernel: Linux 2.6.18-4-k7 Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8) Versions of packages libpam-pgsql depends on: ii libc6 2.3.6.ds1-13 GNU C Library: Shared libraries ii libmhash2 0.9.7-1 Library for cryptographic hashing ii libpam0g 0.79-4 Pluggable Authentication Modules l ii libpq4 8.1.8-1 PostgreSQL C client library libpam-pgsql recommends no packages. -- no debconf information
This patch removes unnecessary SQL escaping of the options table name and
column names. This allows more complex SQL constructs within the
aforementioned options when specified via the configuration file.
-- Fabian Knittel <[EMAIL PROTECTED]>
diff -urN pam-pgsql-0.5.2.orig/pam_pgsql.c pam-pgsql-0.5.2/pam_pgsql.c
--- pam-pgsql-0.5.2.orig/pam_pgsql.c 2007-05-14 18:35:20.000000000 +0200
+++ pam-pgsql-0.5.2/pam_pgsql.c 2007-05-14 20:07:23.000000000 +0200
@@ -425,9 +425,6 @@
PGconn *conn;
int rc, esclen;
char *user_s;
- char *pwd_column_s;
- char *table_s;
- char *user_column_s;
#define CRYPT_LEN 13
if(!(conn = pg_connect(options)))
@@ -439,28 +436,13 @@
sqlescape(user, user_s, strlen(user));
DBGLOG("%s", user_s);
- esclen = strlen(options->pwd_column)*2+1;
- pwd_column_s = malloc(esclen);
- sqlescape(options->pwd_column, pwd_column_s, strlen(options->pwd_column));
-
- esclen = strlen(options->table)*2+1;
- table_s = malloc(esclen);
- sqlescape(options->table, table_s, strlen(options->table));
-
- esclen = strlen(options->user_column)*2+1;
- user_column_s = malloc(esclen);
- sqlescape(options->user_column, user_column_s, strlen(options->user_column));
-
- DBGLOG("query: SELECT %s FROM %s WHERE %s='%s'", pwd_column_s, table_s, user_column_s, user_s);
+ DBGLOG("query: SELECT %s FROM %s WHERE %s='%s'", options->pwd_column, options->table, options->user_column, user_s);
if(pg_exec(options, conn, &res, "SELECT %s FROM %s WHERE %s='%s'",
- pwd_column_s,
- table_s,
- user_column_s,
+ options->pwd_column,
+ options->table,
+ options->user_column,
user_s) != 0) {
free(user_s);
- free(table_s);
- free(user_column_s);
- free(pwd_column_s);
PQclear(res);
PQfinish(conn);
@@ -518,9 +500,6 @@
PQclear(res);
PQfinish(conn);
free(user_s);
- free(table_s);
- free(user_column_s);
- free(pwd_column_s);
return rc;
}
@@ -570,10 +549,6 @@
struct module_options *options;
const char *user;
char *user_s;
- char *table_s;
- char *expired_column_s;
- char *user_column_s;
- char *newtok_column_s;
int rc, esclen;
PGconn *conn;
PGresult *res;
@@ -606,38 +581,17 @@
sqlescape(user, user_s, strlen(user));
- esclen = strlen(options->expired_column)*2+1;
- expired_column_s = malloc(esclen);
- sqlescape(options->expired_column, expired_column_s, strlen(options->expired_column));
-
- esclen = strlen(options->table)*2+1;
- table_s = malloc(esclen);
- sqlescape(options->table, table_s, strlen(options->table));
-
- esclen = strlen(options->user_column)*2+1;
- user_column_s = malloc(esclen);
- sqlescape(options->user_column, user_column_s, strlen(options->user_column));
-
- esclen = strlen(options->newtok_column)*2+1;
- newtok_column_s = malloc(esclen);
- sqlescape(options->newtok_column, newtok_column_s, strlen(options->newtok_column));
-
-
/* if account has expired then expired_column = '1' or 'y' */
if(options->expired_column) {
- DBGLOG("query: SELECT 1 FROM %s WHERE %s='%s' AND %s='y' OR %s='1'", table_s, user_column_s, user_s, expired_column_s, expired_column_s);
+ DBGLOG("query: SELECT 1 FROM %s WHERE %s='%s' AND %s='y' OR %s='1'", options->table, options->user_column, user_s, options->expired_column, options->expired_column);
if(pg_exec(options, conn, &res,
"SELECT 1 FROM %s WHERE %s='%s' AND (%s='y' OR %s='1')" ,
- table_s,
- user_column_s,
+ options->table,
+ options->user_column,
user_s,
- expired_column_s,
- expired_column_s) != 0) {
+ options->expired_column,
+ options->expired_column) != 0) {
free(user_s);
- free(table_s);
- free(newtok_column_s);
- free(user_column_s);
- free(expired_column_s);
PQclear(res);
PQfinish(conn);
@@ -646,10 +600,6 @@
}
if(PQntuples(res) > 0) {
free(user_s);
- free(table_s);
- free(newtok_column_s);
- free(user_column_s);
- free(expired_column_s);
PQclear(res);
PQfinish(conn);
@@ -661,19 +611,15 @@
/* if new password is required then newtok_column = 'y' or '1' */
if(options->newtok_column) {
- DBGLOG("query: SELECT 1 FROM %s WHERE %s='%s' AND %s='y' OR %s='1'", table_s, user_column_s, user_s, newtok_column_s, newtok_column_s);
+ DBGLOG("query: SELECT 1 FROM %s WHERE %s='%s' AND %s='y' OR %s='1'", options->table, options->user_column, user_s, options->newtok_column, options->newtok_column);
if(pg_exec(options, conn, &res,
"SELECT 1 FROM %s WHERE %s='%s' AND (%s='y' OR %s='1')",
- table_s,
- user_column_s,
+ options->table,
+ options->user_column,
user_s,
- newtok_column_s,
- newtok_column_s) != 0) {
+ options->newtok_column,
+ options->newtok_column) != 0) {
free(user_s);
- free(table_s);
- free(newtok_column_s);
- free(user_column_s);
- free(expired_column_s);
PQclear(res);
PQfinish(conn);
@@ -682,10 +628,6 @@
}
if(PQntuples(res) > 0) {
free(user_s);
- free(table_s);
- free(newtok_column_s);
- free(user_column_s);
- free(expired_column_s);
PQclear(res);
PQfinish(conn);
@@ -697,10 +639,6 @@
PQfinish(conn);
free(user_s);
- free(table_s);
- free(newtok_column_s);
- free(user_column_s);
- free(expired_column_s);
free_module_options(options);
return PAM_SUCCESS;
}
@@ -713,11 +651,7 @@
int rc, std_flags, esclen;
const char *user, *pass, *newpass;
char *newpass_crypt, *user_s;
- char *table_s;
char *newpass_crypt_s;
- char *user_column_s;
- char *pwd_column_s;
- char *newtok_column_s;
PGconn *conn;
PGresult *res;
@@ -811,44 +745,23 @@
sqlescape(user, user_s, strlen(user));
- esclen = strlen(options->pwd_column)*2+1;
- pwd_column_s = malloc(esclen);
- sqlescape(options->pwd_column, pwd_column_s, strlen(options->pwd_column));
-
- esclen = strlen(options->table)*2+1;
- table_s = malloc(esclen);
- sqlescape(options->table, table_s, strlen(options->table));
-
- esclen = strlen(options->user_column)*2+1;
- user_column_s = malloc(esclen);
- sqlescape(options->user_column, user_column_s, strlen(options->user_column));
-
esclen = strlen(newpass_crypt)*2+1;
newpass_crypt_s = malloc(esclen);
sqlescape(newpass_crypt, newpass_crypt_s, strlen(newpass_crypt));
- esclen = strlen(options->newtok_column)*2+1;
- newtok_column_s = malloc(esclen);
- sqlescape(options->newtok_column, newtok_column_s, strlen(options->newtok_column));
-
-
- DBGLOG("query: UPDATE %s SET %s='%s' WHERE %s='%s'", table_s, pwd_column_s, "******", user_column_s, user_s);
+ DBGLOG("query: UPDATE %s SET %s='%s' WHERE %s='%s'", options->table, options->pwd_column, "******", options->user_column, user_s);
if (!options->newtok_column) {
if(pg_exec(options, conn, &res,
"UPDATE %s SET %s='%s' WHERE %s='%s'",
- table_s,
- pwd_column_s,
+ options->table,
+ options->pwd_column,
newpass_crypt_s,
- user_column_s,
+ options->user_column,
user_s) != 0) {
free(newpass_crypt);
free(newpass_crypt_s);
free(user_s);
- free(pwd_column_s);
- free(user_column_s);
- free(newtok_column_s);
- free(table_s);
free_module_options(options);
PQclear(res);
PQfinish(conn);
@@ -857,19 +770,14 @@
} else {
if(pg_exec(options, conn, &res,
"UPDATE %s SET %s='%s', %s='0' WHERE %s='%s'",
- table_s,
- pwd_column_s,
+ options->table,
+ options->pwd_column,
newpass_crypt_s,
- newtok_column_s,
- user_column_s,
+ options->user_column,
user_s) != 0) {
free(newpass_crypt);
free(newpass_crypt_s);
free(user_s);
- free(pwd_column_s);
- free(user_column_s);
- free(newtok_column_s);
- free(table_s);
free_module_options(options);
PQclear(res);
PQfinish(conn);
@@ -881,10 +789,6 @@
free(newpass_crypt);
free(newpass_crypt_s);
free(user_s);
- free(pwd_column_s);
- free(user_column_s);
- free(newtok_column_s);
- free(table_s);
PQclear(res);
PQfinish(conn);
}
signature.asc
Description: Digital signature
