Core dump on sql config errors

Maurice Makaay <[email protected]> Tue, 4 Nov 2003 10:13:34 +0100
Newsgroups gmane.comp.gnu.radius.bugs
Organization InterNLnet BV
Message-ID <[email protected]>
Hi,

If certain keywords are omitted from the sqlserver configuration, the
server will crash with SEGFAULT on startup. E.g. if acct_db is not set
(even if do_acct is set to false) the server will crash. In the case 
sql debugging is enabled, the server will trip over the statement
debug(1, ("SQL init using: %s:%d,%s,%s,%s,%d,%ld,%d,%d",....) in 
sql_check_config() which will try to print out NULL values.

In the attached patch, the sql_check_config() is extended to have 
better checking of the sql configuration and to provide defaults for
variables which are not set in the sqlserver config file. Each missing
parameter will be logged at the L_WARN loglevel. The setting of default
values could be done at the start of rad_sql_init(), but I think it's 
better to do it in sql_check_config(), so warning messages can be
spawned to the user.

Regards,

-- Maurice Makaay

_______________________________________________
Bug-gnu-radius mailing list
[email protected]
http://mail.gnu.org/mailman/listinfo/bug-gnu-radius
patch.config.sql (text/plain, 2.3 KB)
Index: sql.c
===================================================================
RCS file: /cvsroot/radius/radius/radiusd/sql.c,v
retrieving revision 1.66
diff -u -r1.66 sql.c
--- sql.c	14 Oct 2003 07:10:43 -0000	1.66
+++ sql.c	4 Nov 2003 09:01:47 -0000
@@ -523,6 +523,12 @@
                                _("SQL auth: no group_query specified"));
                 }
                 FREE_IF_EMPTY(cfg->group_query);
+		if (!cfg->auth_db) {
+			radlog(L_WARN,
+		           _("SQL: missing parameter '%s', reverting to '%s'"),
+			   "auth_db", "radius");
+			cfg->auth_db = "radius";
+		}
         }
         /*
          * Check if we should do SQL accounting
@@ -552,14 +558,55 @@
                      _("SQL acct: no acct_nasup_query specified"));
                 }
                 FREE_IF_EMPTY(cfg->acct_nasup_query);
+		if (!cfg->acct_db) {
+			radlog(L_WARN,
+		           _("SQL: missing parameter '%s', reverting to '%s'"),
+			   "acct_db", "radius");
+			cfg->acct_db = "radius";
+		}
         }
+
+	/*
+	 * Check if all needed database parameters are set. Apply defaults
+	 * if they are not.
+	 */
+	if (!cfg->server) {
+		radlog(L_WARN,
+		       _("SQL: missing parameter '%s', reverting to '%s'"),
+		       "server", "localhost");
+		cfg->server = "localhost";
+	}
+	if (!cfg->port) {
+		radlog(L_WARN,
+		       _("SQL: missing parameter '%s', reverting to '%s'"),
+		       "port", "3306");
+		cfg->port = 3306;
+	}
+	if (!cfg->login) {
+		radlog(L_WARN,
+		       _("SQL: missing parameter '%s', reverting to '%s'"),
+		       "login", "radius");
+		cfg->login = "radius";
+	}
+	if (!cfg->password) {
+		radlog(L_WARN,
+		       _("SQL: missing parameter '%s', reverting to '%s'"),
+		       "login", "radius");
+		cfg->password = "radius";
+	}
+	if (!cfg->password) {
+		radlog(L_WARN,
+		       _("SQL: missing parameter '%s', reverting to '%s'"),
+		       "login", "radiusd");
+		cfg->password = "radiusd";
+	}
         
         debug(1, ("SQL init using: %s:%d,%s,%s,%s,%d,%ld,%d,%d",
                cfg->server,
                cfg->port,
                cfg->login,
-               cfg->acct_db,
-               cfg->auth_db,
+               (cfg->acct_db ? cfg->acct_db : "none"),
+               (cfg->auth_db ? cfg->auth_db : "none"),
                cfg->keepopen,
                cfg->idle_timeout,
                cfg->doacct,