--- Begin Message ---
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
--- End Message ---
