Resize the buffer for the SQL-User-Name, and update the

	comments to help people (...) to figure out why there is
	no escaping in sql_escape_func().

diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c
index ab63d29..dc8592b 100644 (file)
--- a/src/modules/rlm_sql/rlm_sql.c
+++ b/src/modules/rlm_sql/rlm_sql.c
@@ -165,14 +165,14 @@ static int sql_escape_func(char *out, int outlen, const char *in);
  *     the first element of the SELECT result will be used.
  */
 static int sql_xlat(void *instance, REQUEST *request,
-                   char *fmt, char *out, int freespace,
+                   char *fmt, char *out, size_t freespace,
                    RADIUS_ESCAPE_STRING func)
 {
        SQLSOCK *sqlsocket;
        SQL_ROW row;
        SQL_INST *inst = instance;
        char querystr[MAX_QUERY_LEN];
-       char sqlusername[2 * MAX_STRING_LEN + 10];
+       char sqlusername[MAX_STRING_LEN];
        int ret = 0;
 
        DEBUG("rlm_sql (%s): - sql_xlat", inst->config->xlat_name);
@@ -429,15 +429,20 @@ static int sql_escape_func(char *out, int outlen, const char *in)
 }
 
 /*
- *     Set the SQl user name.
+ *     Set the SQL user name.
+ *
+ *     We don't call the escape function here. The resulting string
+ *     will be escaped later in the queries xlat so we don't need to
+ *     escape it twice. (it will make things wrong if we have an
+ *     escape candidate character in the username)
  */
 static int sql_set_user(SQL_INST *inst, REQUEST *request, char *sqlusername, const char *username)
 {
        VALUE_PAIR *vp=NULL;
        char tmpuser[MAX_STRING_LEN];
 
-       tmpuser[0]=0;
-       sqlusername[0]=0;
+       tmpuser[0] = '\0';
+       sqlusername[0]= '\0';
 
        /* Remove any user attr we added previously */
        pairdelete(&request->packet->vps, PW_SQL_USER_NAME);
@@ -451,7 +456,7 @@ static int sql_set_user(SQL_INST *inst, REQUEST *request, char *sqlusername, con
        }
 
        if (*tmpuser) {
-               strNcpy(sqlusername, tmpuser, sizeof(tmpuser));
+               strNcpy(sqlusername, tmpuser, MAX_STRING_LEN);
                DEBUG2("rlm_sql (%s): sql_set_user escaped user --> '%s'",
                       inst->config->xlat_name, sqlusername);
                vp = pairmake("SQL-User-Name", sqlusername, 0);
@@ -545,7 +550,7 @@ static int sql_groupcmp(void *instance, REQUEST *req, VALUE_PAIR *request, VALUE
 {
        SQLSOCK *sqlsocket;
        SQL_INST *inst = instance;
-       char sqlusername[2 * MAX_STRING_LEN + 10];
+       char sqlusername[MAX_STRING_LEN];
        SQL_GROUPLIST *group_list, *group_list_tmp;
 
        check_pairs = check_pairs;
@@ -566,7 +571,7 @@ static int sql_groupcmp(void *instance, REQUEST *req, VALUE_PAIR *request, VALUE
        /*
         * Set, escape, and check the user attr here
         */
-       if (sql_set_user(inst, req, sqlusername, 0) < 0)
+       if (sql_set_user(inst, req, sqlusername, NULL) < 0)
                return 1;
 
        /*
@@ -805,7 +810,7 @@ static int rlm_sql_detach(void *instance)
 static int rlm_sql_instantiate(CONF_SECTION * conf, void **instance)
 {
        SQL_INST *inst;
-       char *xlat_name;
+       const char *xlat_name;
 
        inst = rad_malloc(sizeof(SQL_INST));
        memset(inst, 0, sizeof(SQL_INST));
@@ -907,19 +912,13 @@ static int rlm_sql_authorize(void *instance, REQUEST * request)
        SQLSOCK *sqlsocket;
        SQL_INST *inst = instance;
        char    querystr[MAX_QUERY_LEN];
-
-       /* sqlusername holds the sql escaped username. The original
-        * username is at most MAX_STRING_LEN chars long and
-        * *sql_escape_string doubles its length in the worst case.
-        * Throw in an extra 10 to account for trailing NULs and to have
-        * a safety margin. */
-       char   sqlusername[2 * MAX_STRING_LEN + 10];
+       char    sqlusername[MAX_STRING_LEN];
        /*
         * the profile username is used as the sqlusername during
         * profile checking so that we don't overwrite the orignal
         * sqlusername string
         */
-       char   profileusername[2 * MAX_STRING_LEN + 10];
+       char   profileusername[MAX_STRING_LEN];
 
        /*
         *      They MUST have a user name to do SQL authorization.
@@ -1106,7 +1105,7 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) {
        int     acctstatustype = 0;
        char    querystr[MAX_QUERY_LEN];
        char    logstr[MAX_QUERY_LEN];
-       char    sqlusername[2 * MAX_STRING_LEN + 10];
+       char    sqlusername[MAX_STRING_LEN];
 
 #ifdef CISCO_ACCOUNTING_HACK
        int     acctsessiontime = 0;
@@ -1343,7 +1342,7 @@ static int rlm_sql_checksimul(void *instance, REQUEST * request) {
        SQL_INST        *inst = instance;
        SQL_ROW         row;
        char            querystr[MAX_QUERY_LEN];
-       char            sqlusername[2*MAX_STRING_LEN+10];
+       char            sqlusername[MAX_STRING_LEN];
        int             check = 0;
         uint32_t        ipno = 0;
         char            *call_num = NULL;
@@ -1363,7 +1362,7 @@ static int rlm_sql_checksimul(void *instance, REQUEST * request) {
        }
 
 
-       if(sql_set_user(inst, request, sqlusername, 0) <0)
+       if(sql_set_user(inst, request, sqlusername, NULL) < 0)
                return RLM_MODULE_FAIL;
 
        radius_xlat(querystr, sizeof(querystr), inst->config->simul_count_query, request, sql_escape_func);
@@ -1520,11 +1519,11 @@ static int rlm_sql_postauth(void *instance, REQUEST *request) {
        SQLSOCK         *sqlsocket = NULL;
        SQL_INST        *inst = instance;
        char            querystr[MAX_QUERY_LEN];
-       char            sqlusername[2*MAX_STRING_LEN+10];
+       char            sqlusername[MAX_STRING_LEN];
 
        DEBUG("rlm_sql (%s): Processing sql_postauth", inst->config->xlat_name);
 
-       if(sql_set_user(inst, request, sqlusername, 0) <0)
+       if(sql_set_user(inst, request, sqlusername, NULL) < 0)
                return RLM_MODULE_FAIL;
 
        /* If postauth_query is not defined, we stop here */