Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Safety check for wms server filters
  • Loading branch information
mhugent committed May 11, 2011
1 parent 3cc052d commit cba683f
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 1 deletion.
76 changes: 75 additions & 1 deletion src/mapserver/qgswmsserver.cpp
Expand Up @@ -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 )
{
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
3 changes: 3 additions & 0 deletions src/mapserver/qgswmsserver.h
Expand Up @@ -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;
Expand Down

0 comments on commit cba683f

Please sign in to comment.