Safety check for wms server filters

mhugent · mhugent · commit cba683f6710c · 2011-05-11T09:39:15.000+02:00
diff --git a/src/mapserver/qgswmsserver.cpp b/src/mapserver/qgswmsserver.cpp
@@ -1522,7 +1522,7 @@ QMap<QString, QString> QgsWMSServer::applyRequestedLayerFilters( const QStringLi
   if ( filterIt != mParameterMap.end() )
   {
     QString filterParameter = filterIt->second;
-    QStringList layerSplit = filterParameter.split( "," );
+    QStringList layerSplit = filterParameter.split( ";" );
     QStringList::const_iterator layerIt = layerSplit.constBegin();
     for ( ; layerIt != layerSplit.constEnd(); ++layerIt )
     {
@@ -1532,6 +1532,15 @@ QMap<QString, QString> QgsWMSServer::applyRequestedLayerFilters( const QStringLi
         continue;
       }
 
+      //filter string could be unsafe (danger of sql injection)
+      if ( !testFilterStringSafety( eqSplit.at( 1 ) ) )
+      {
+        throw QgsMapServiceException( "Filter string rejected", "The filter string " + eqSplit.at( 1 ) +
+                                      " has been rejected because of security reasons. Note: Text strings have to be enclosed in single or double quotes. " +
+                                      "A space between each word / special character is mandatory. Allowed Keywords and special characters are " +
+                                      "AND,OR,IN,<,>=,>,>=,!=,',',(,). Not allowed are semicolons in the filter expression." );
+      }
+
       //we know the layer name, but need to go through the list because a layer could be there several times...
       int listPos = 1;
       QStringList::const_iterator layerIt = layerList.constBegin();
@@ -1581,3 +1590,68 @@ void QgsWMSServer::restoreLayerFilters( const QMap < QString, QString >& filterM
     }
   }
 }
+
+bool QgsWMSServer::testFilterStringSafety( const QString& filter ) const
+{
+  //; too dangerous for sql injections
+  if ( filter.contains( ";" ) )
+  {
+    return false;
+  }
+
+  QStringList tokens = filter.split( " ", QString::SkipEmptyParts );
+  QStringList::const_iterator tokenIt = tokens.constBegin();
+  for ( ; tokenIt != tokens.constEnd(); ++tokenIt )
+  {
+    //whitelist of allowed characters and keywords
+    if ( tokenIt->compare( "," ) == 0
+         || tokenIt->compare( "(" ) == 0
+         || tokenIt->compare( ")" ) == 0
+         || tokenIt->compare( "=" ) == 0
+         || tokenIt->compare( "!=" ) == 0
+         || tokenIt->compare( "<" ) == 0
+         || tokenIt->compare( "<=" ) == 0
+         || tokenIt->compare( ">" ) == 0
+         || tokenIt->compare( ">=" ) == 0
+         || tokenIt->compare( "AND", Qt::CaseInsensitive ) == 0
+         || tokenIt->compare( "OR", Qt::CaseInsensitive ) == 0
+         || tokenIt->compare( "IN", Qt::CaseInsensitive ) == 0 )
+    {
+      continue;
+    }
+
+    //numbers are ok
+    bool isNumeric;
+    tokenIt->toDouble( &isNumeric );
+    if ( isNumeric )
+    {
+      continue;
+    }
+
+    //numeric strings need to be quoted once either with single or with double quotes
+
+    //single quote
+    if ( tokenIt->size() > 2
+         && ( *tokenIt )[0] == QChar( '\'' )
+         && ( *tokenIt )[tokenIt->size() - 1] == QChar( '\'' )
+         && ( *tokenIt )[1] != QChar( '\'' )
+         && ( *tokenIt )[tokenIt->size() - 2] != QChar( '\'' ) )
+    {
+      continue;
+    }
+
+    //double quote
+    if ( tokenIt->size() > 2
+         && ( *tokenIt )[0] == QChar( '"' )
+         && ( *tokenIt )[tokenIt->size() - 1] == QChar( '"' )
+         && ( *tokenIt )[1] != QChar( '"' )
+         && ( *tokenIt )[tokenIt->size() - 2] != QChar( '"' ) )
+    {
+      continue;
+    }
+
+    return false;
+  }
+
+  return true;
+}
diff --git a/src/mapserver/qgswmsserver.h b/src/mapserver/qgswmsserver.h
@@ -143,6 +143,9 @@ class QgsWMSServer
     QMap<QString, QString> applyRequestedLayerFilters( const QStringList& layerList, const QStringList& layerIds ) const;
     /**Restores the original layer filters*/
     void restoreLayerFilters( const QMap < QString, QString >& filterMap ) const;
+    /**Tests if a filter sql string is allowed (safe)
+      @return true in case of success, false if string seems unsafe*/
+    bool testFilterStringSafety( const QString& filter ) const;
 
     /**Map containing the WMS parameters*/
     std::map<QString, QString> mParameterMap;
