Require the SQL used by ODBCAppender to be a single statement (#564)

* Add web-site nudge to protect the configuration file

Author: swebb2066

src/main/cpp/odbcappender.cpp

@@ -267,6 +267,7 @@ void ODBCAppender::append(const spi::LoggingEventPtr& event, LOG4CXX_NS::helpers
 #endif
 }
 
+#if LOG4CXX_ABI_VERSION <= 15
 LogString ODBCAppender::getLogStatement(const spi::LoggingEventPtr& event, LOG4CXX_NS::helpers::Pool& p) const
 {
     return LogString();
@@ -275,6 +276,7 @@ LogString ODBCAppender::getLogStatement(const spi::LoggingEventPtr& event, LOG4C
 void ODBCAppender::execute(const LogString& sql, LOG4CXX_NS::helpers::Pool& p)
 {
 }
+#endif
 
 /* The default behavior holds a single connection open until the appender
 is closed (typically when garbage collected).*/
@@ -613,6 +615,28 @@ void ODBCAppender::flushBuffer(Pool& p)
 
 void ODBCAppender::setSql(const LogString& s)
 {
+	const logchar doubleQuote{ 0x22 };
+	const logchar singleQuote{ 0x27 };
+	const logchar semiColan{ 0x3b };
+	// A basic check which disallows multiple SQL statements - for defense-in-depth security.
+	// Allow a semicolan in a quoted context or as the last character.
+	logchar currentQuote{ 0 };
+	int charCount{ 0 };
+	for (auto ch : s)
+	{
+		++charCount;
+		if (currentQuote == ch)
+			currentQuote = 0;
+		else if (currentQuote == 0)
+		{
+			if (doubleQuote == ch || singleQuote == ch)
+				currentQuote = ch;
+			else if (semiColan == ch && s.size() != charCount)
+				throw IllegalArgumentException(LOG4CXX_STR("SQL statement cannot contain a ';'"));
+		}
+	}
+	if (0 != currentQuote)
+		throw IllegalArgumentException(LogString(LOG4CXX_STR("Unmatched ")) + currentQuote + LOG4CXX_STR(" in SQL statement"));
     _priv->sqlStatement = s;
 }
 

src/main/include/log4cxx/db/odbcappender.h

@@ -187,6 +187,7 @@ class LOG4CXX_EXPORT ODBCAppender : public AppenderSkeleton
 		void append(const spi::LoggingEventPtr& event, helpers::Pool&) override;
 
 	protected:
+#if LOG4CXX_ABI_VERSION <= 15
 		/**
 		* To be removed.
 		*/
@@ -199,7 +200,7 @@ class LOG4CXX_EXPORT ODBCAppender : public AppenderSkeleton
 		* */
 		virtual void execute(const LogString& sql,
 			LOG4CXX_NS::helpers::Pool& p) /*throw(SQLException)*/;
-
+#endif
 		/**
 		* Override this to return the connection to a pool, or to clean up the
 		* resource.

src/site/markdown/configuration-samples.md

@@ -27,6 +27,11 @@ specify the instantiated appender/layout/filter classes and
 the properties of those class instances
 without recompiling and rebuilding.
 
+The configuration file must be protected from modification by untrusted parties.
+Use restrictive file system permissions to ensure
+untrusted parties do not have write access.
+Do not load the configuration file from an untrusted location.
+
 As Log4cxx was designed to be extendable,
 property names and values are not constrained by the core library.
 The configuration file parsers,

src/test/cpp/db/odbcappendertestcase.cpp

@@ -39,7 +39,10 @@ class ODBCAppenderTestCase : public AppenderSkeletonTestCase
 		//
 		LOGUNIT_TEST(testDefaultThreshold);
 		LOGUNIT_TEST(testSetOptionThreshold);
-		//LOGUNIT_TEST(testConnectUsingDSN);
+//#define DataSourceName_Log4cxxTest_Is_Valid
+#ifdef DataSourceName_Log4cxxTest_Is_Valid
+		LOGUNIT_TEST(testConnectUsingDSN);
+#endif
 		LOGUNIT_TEST_SUITE_END();
 
 
@@ -72,7 +75,7 @@ class ODBCAppenderTestCase : public AppenderSkeletonTestCase
 //
 // CREATE TABLE [dbo].[UnitTestLog](
 //	 [Item] [bigint] IDENTITY(1,1) NOT NULL, /* auto incremented */
-//	 [Thread] [nchar](20) NULL
+//	 [Thread] [nchar](20) NULL,
 //	 [LogTime] [datetime] NOT NULL,
 //	 [LogName] [nchar](50) NULL,
 //	 [LogLevel] [nchar](10) NULL,
@@ -90,7 +93,7 @@ class ODBCAppenderTestCase : public AppenderSkeletonTestCase
 			for (int i = 0; i < 100; ++i)
 			{
 				LOG4CXX_INFO(odbc, "Message '" << i << "'");
-				apr_sleep(30000);
+				apr_sleep(30000); // 30 milliseconds
 			}
 			LOG4CXX_INFO(odbc, "Last message");
 		}