Merge pull request #77 from alorbach/pr-issue-76

login: Fix XSS issue if "Debug Userlogin" is enabled.
rsyslog · Apr 29, 2021 · a0cb246 · a0cb246
2 parents 5d0247e + 28ae0b9
commit a0cb246
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 20 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,4 +1,10 @@
 ---------------------------------------------------------------------------
+Version 4.1.12 (stable), 2021-04-29
+---------------------------------------------------------------------------
+- Secured username field against XSS attacks, thanks for reporting to:
+  Michael Strametz of SySS Cyber Security GmbH (Austria).
+- UserDB: Allow NULL value for defaultfilter fields, updated to v13
+---------------------------------------------------------------------------
 Version 4.1.11 (stable), 2020-07-09
 - ThirdParty: Updated jpgraph to 4.3.1 (2020-04-24)
 - Thanks to Javier Pastor for the following fixes and changes: 

diff --git a/src/include/db_template.txt b/src/include/db_template.txt
@@ -81,7 +81,7 @@ CREATE TABLE `logcon_sources` (
   `DBTableName` varchar(64) default NULL,
   `DBEnableRowCounting` tinyint(1) default NULL,
   `DBRecordsPerQuery` int(11) NOT NULL default '100',
-  `defaultfilter` VARCHAR(1024) NOT NULL, 
+  `defaultfilter` VARCHAR(1024) NULL, 
   `userid` int(11) default NULL,
   `groupid` int(11) default NULL,
   PRIMARY KEY  (`ID`)
@@ -128,7 +128,7 @@ CREATE TABLE IF NOT EXISTS `logcon_charts` (
   `chart_type` int(11) NOT NULL,
   `chart_width` int(11) NOT NULL,
   `chart_field` varchar(255) NOT NULL,
-  `chart_defaultfilter` VARCHAR(1024) NOT NULL, 
+  `chart_defaultfilter` VARCHAR(1024) NULL, 
   `maxrecords` int(11) NOT NULL,
   `showpercent` tinyint(1) NOT NULL,
   `userid` int(11) default NULL,

diff --git a/src/include/db_update_v13.txt b/src/include/db_update_v13.txt
@@ -0,0 +1,7 @@
+-- New Database Structure Updates
+ALTER TABLE `logcon_sources` CHANGE `defaultfilter` `defaultfilter` VARCHAR(1024) NULL;
+ALTER TABLE `logcon_charts` CHANGE `chart_defaultfilter` `chart_defaultfilter` VARCHAR(1024) NULL;
+
+-- Insert data
+
+-- Updated Data
diff --git a/src/include/functions_common.php b/src/include/functions_common.php
@@ -65,7 +65,7 @@
 $LANG = "en";		// Default language
 
 // Default Template vars
-$content['BUILDNUMBER'] = "4.1.11";
+$content['BUILDNUMBER'] = "4.1.12";
 $content['UPDATEURL'] = "http://loganalyzer.adiscon.com/files/version.txt";
 $content['TITLE'] = "Adiscon LogAnalyzer :: Release " . $content['BUILDNUMBER'];	// Default page title 
 $content['BASEPATH'] = $gl_root_path;

diff --git a/src/include/functions_db.php b/src/include/functions_db.php
@@ -49,7 +49,7 @@
 $errno = 0;
 
 // --- Current Database Version, this is important for automated database Updates!
-$content['database_internalversion'] = "12";	// Whenever incremented, a database upgrade is needed
+$content['database_internalversion'] = "13";	// Whenever incremented, a database upgrade is needed
 $content['database_installedversion'] = "0";	// 0 is default which means Prior Versioning Database
 // --- 
 

diff --git a/src/include/functions_users.php b/src/include/functions_users.php
@@ -283,7 +283,7 @@ function CheckUserLogin( $username, $password )
 		}
 		*/
 		if ( GetConfigSetting("DebugUserLogin", 0) == 1 )
-			DieWithFriendlyErrorMsg( "Debug Error: Could not find user '" . $username . "' <br><br><B>Sessionarray</B> <pre>" . var_export($_SESSION, true) . "</pre>");
+			DieWithFriendlyErrorMsg( "Debug Error: Could not find user '" . htmlspecialchars($username) . "' <br><br><B>Sessionarray</B> <pre>" . var_export($_SESSION, true) . "</pre>");
 
 		// Default return false
 		return false;

diff --git a/src/login.php b/src/login.php
@@ -65,38 +65,32 @@
 	$szRedir = "index.php"; // Default
 $szRedir = SecureRedirect($szRedir); 
 
-if ( isset($_POST['op']) && $_POST['op'] == "login" )
-{
+if ( isset($_POST['op']) && $_POST['op'] == "login" ) {
 	// Perform login!
 	if ( $_POST['op'] == "login" )
 	{
 		if ( 
 			 (isset($_POST['uname']) && strlen($_POST['uname']) > 0) 
 				&& 
 			 (isset($_POST['pass']) && strlen($_POST['pass']) > 0)
-			)
-		{
-			// Set Username and password
-			$content['uname'] = DB_RemoveBadChars($_POST['uname']);
-			$content['pass'] = $_POST['pass']; // RAW Copy of password string, otherwise passwords with special characters can be broken. 
+			) {
+			// Copy Username and password for template system
+			$content['uname'] = htmlspecialchars(DB_RemoveBadChars($_POST['uname'])); // URL Decode the username to avoid XSS issues!
+			$content['pass'] = htmlspecialchars($_POST['pass']); // RAW Copy of password string, otherwise passwords with special characters can be broken.
 
-			if ( !CheckUserLogin( $content['uname'], $content['pass']) )
-			{
+			// Use raw properties for database login check
+			if ( !CheckUserLogin( DB_RemoveBadChars($_POST['uname']), $_POST['pass']) ) {
 				$content['ISERROR'] = "true";
 				$content['ERROR_MSG'] = $content['LN_LOGIN_ERRWRONGPASSWORD'];
 			}
 			else
 				RedirectPage( urldecode($szRedir) );
-		}
-		else
-		{
+		} else {
 			$content['ISERROR'] = "true";
 			$content['ERROR_MSG'] = $content['LN_LOGIN_USERPASSMISSING'];
 		}
 	}
-}
-else if ( isset($_GET['op']) && $_GET['op'] == "logoff" )
-{
+} else if ( isset($_GET['op']) && $_GET['op'] == "logoff" ) {
 	// logoff in this case
 	DoLogOff();
 }