Quels sont les dangers du SQL dynamique et peuvent-ils être évités?
-
06-07-2019 - |
Question
Nous venons de recevoir le code suivant comme solution pour une requête de recherche complexe dans une nouvelle application fournie par des développeurs offshore. Je suis sceptique quant à l'utilisation de SQL dynamique car je pourrais fermer l'instruction SQL avec '; et ensuite exécuter un méchant qui sera effectué sur la base de données!
Des idées sur la façon de réparer l'attaque par injection?
ALTER procedure [dbo].[SearchVenues] --'','',10,1,1,''
@selectedFeature as varchar(MAX),
@searchStr as varchar(100),
@pageCount as int,
@startIndex as int,
@searchId as int,
@venueName as varchar(100),
@range int,
@latitude varchar(100),
@longitude varchar(100),
@showAll int,
@OrderBy varchar(50),
@SearchOrder varchar(10)
AS
DECLARE @sqlRowNum as varchar(max)
DECLARE @sqlRowNumWhere as varchar(max)
DECLARE @withFunction as varchar(max)
DECLARE @withFunction1 as varchar(max)
DECLARE @endIndex as int
SET @endIndex = @startIndex + @pageCount -1
SET @sqlRowNum = ' SELECT Row_Number() OVER (ORDER BY '
IF @OrderBy = 'Distance'
SET @sqlRowNum = @sqlRowNum + 'dbo.GeocodeDistanceMiles(Latitude,Longitude,' + @latitude + ',' + @longitude + ') ' +@SearchOrder
ELSE
SET @sqlRowNum = @sqlRowNum + @OrderBy + ' '+ @SearchOrder
SET @sqlRowNum = @sqlRowNum + ' ) AS RowNumber,ID,RecordId,EliteStatus,Name,Description,
Address,TotalReviews,AverageFacilityRating,AverageServiceRating,Address1,Address2,Address3,Address4,Address5,Address6,PhoneNumber,
visitCount,referalCount,requestCount,imgUrl,Latitude,Longitude,
Convert(decimal(10,2),dbo.GeocodeDistanceMiles(Latitude,Longitude,' + @latitude + ',' + @longitude + ')) as distance
FROM VenueAllData '
SET @sqlRowNumWhere = 'where Enabled=1 and EliteStatus <> 3 '
--PRINT('@sqlRowNum ='+@sqlRowNum)
IF @searchStr <> ''
BEGIN
IF (@searchId = 1) -- county search
BEGIN
SET @sqlRowNumWhere = @sqlRowNumWhere + ' and Address5 like ''' + @searchStr + '%'''
END
ELSE IF(@searchId = 2 ) -- Town search
BEGIN
SET @sqlRowNumWhere = @sqlRowNumWhere + ' and Address4 like ''' + @searchStr + '%'''
END
ELSE IF(@searchId = 3 ) -- postcode search
BEGIN
SET @sqlRowNumWhere = @sqlRowNumWhere + ' and Address6 like ''' + @searchStr + '%'''
END
IF (@searchId = 4) -- Search By Name
BEGIN
IF @venueName <> ''
SET @sqlRowNumWhere = @sqlRowNumWhere + ' and ( Name like ''%' + @venueName + '%'' OR Address like ''%'+ @venueName+'%'' ) '
ELSE
SET @sqlRowNumWhere = @sqlRowNumWhere + ' and ( Name like ''%' + @searchStr + '%'' OR Address like ''%'+ @searchStr+'%'' ) '
END
END
IF @venueName <> '' AND @searchId <> 4
SET @sqlRowNumWhere = @sqlRowNumWhere + ' and ( Name like ''%' + @venueName + '%'' OR Address like ''%'+ @venueName+'%'' ) '
set @sqlRowNum = @sqlRowNum + ' ' + @sqlRowNumWhere
--PRINT(@sqlRowNum)
IF @selectedFeature <> ''
BEGIN
DECLARE @val1 varchar (255)
Declare @SQLAttributes varchar(max)
Set @SQLAttributes = ''
Declare @tempAttribute varchar(max)
Declare @AttrId int
while (@selectedFeature <> '')
BEGIN
SET @AttrId = CAST(SUBSTRING(@selectedFeature,1,CHARINDEX(',',@selectedFeature)-1) AS Int)
Select @tempAttribute = ColumnName from Attribute where id = @AttrId
SET @selectedFeature = SUBSTRING(@selectedFeature,len(@AttrId)+2,len(@selectedFeature))
SET @SQLAttributes = @SQLAttributes + ' ' + @tempAttribute + ' = 1 And '
END
Set @SQLAttributes = SUBSTRING(@SQLAttributes,0,LEN(@SQLAttributes)-3)
set @sqlRowNum = @sqlRowNum + ' and ID in (Select VenueId from '
set @sqlRowNum = @sqlRowNum + ' CachedVenueAttributes WHERE ' + @SQLAttributes + ') '
END
IF @showAll <> 1
set @sqlRowNum = @sqlRowNum + ' and dbo.GeocodeDistanceMiles(Latitude,Longitude,' + @latitude + ',' + @longitude + ') <= ' + convert(varchar,@range )
set @withFunction = 'WITH LogEntries AS (' + @sqlRowNum + ')
SELECT * FROM LogEntries WHERE RowNumber between '+ Convert(varchar,@startIndex) +
' and ' + Convert(varchar,@endIndex) + ' ORDER BY ' + @OrderBy + ' ' + @SearchOrder
print(@withFunction)
exec(@withFunction)
La solution
En passant, je n’utiliserais pas EXEC
; J'utiliserais plutôt sp_executesql
. Consultez ce superbe article, La malédiction et les bénédictions du SQL dynamique , pour connaître le motif et d'autres informations sur en utilisant SQL dynamique.
Autres conseils
Voici une version optimisée de la requête ci-dessus qui n’utilise pas de SQL dynamique ...
Declare @selectedFeature as varchar(MAX),
@searchStr as varchar(100),
@pageCount as int,
@startIndex as int,
@searchId as int,
@venueName as varchar(100),
@range int,
@latitude varchar(100),
@longitude varchar(100),
@showAll int,
@OrderBy varchar(50),
@SearchOrder varchar(10)
Set @startIndex = 1
Set @pageCount = 50
Set @searchStr = 'e'
Set @searchId = 4
Set @OrderBy = 'Address1'
Set @showAll = 1
--Select dbo.GeocodeDistanceMiles(Latitude,Longitude,@latitude,@longitude)
DECLARE @endIndex int
SET @endIndex = @startIndex + @pageCount -1
;
WITH LogEntries as (
SELECT
Row_Number()
OVER (ORDER BY
CASE @OrderBy
WHEN 'Distance' THEN Cast(dbo.GeocodeDistanceMiles(Latitude,Longitude,@latitude,@longitude) as varchar(10))
WHEN 'Name' THEN Name
WHEN 'Address1' THEN Address1
WHEN 'RecordId' THEN Cast(RecordId as varchar(10))
WHEN 'EliteStatus' THEN Cast(EliteStatus as varchar(10))
END) AS RowNumber,
RecordId,EliteStatus,Name,Description,
Address,TotalReviews,AverageFacilityRating,AverageServiceRating,Address1,Address2,Address3,Address4,Address5,Address6,PhoneNumber,
visitCount,referalCount,requestCount,imgUrl,Latitude,Longitude,
Convert(decimal(10,2),dbo.GeocodeDistanceMiles(Latitude,Longitude,@latitude,@longitude)) as distance
FROM VenueAllData
where Enabled=1 and EliteStatus <> 3
And
(
(Address5 like @searchStr + '%' And @searchId = 1) OR
(Address4 like @searchStr + '%' And @searchId = 2) OR
(Address6 like @searchStr + '%' And @searchId = 3) OR
(
(
@searchId = 4 And
(Name like '%' + @venueName + '%' OR Address like '%'+ @searchStr+'%')
)
)
)
And
ID in (
Select VenueID
From CachedVenueAttributes
--Extra Where Clause for the processing of VenueAttributes using @selectedFeature
)
And
(
(@showAll = 1) Or
(@showAll <> 1 and dbo.GeocodeDistanceMiles(Latitude,Longitude,@latitude,@longitude) <= convert(varchar,@range ))
)
)
SELECT * FROM LogEntries
WHERE RowNumber between @startIndex and @endIndex
ORDER BY CASE @OrderBy
WHEN 'Distance' THEN Cast(Distance as varchar(10))
WHEN 'Name' THEN Name
WHEN 'Address1' THEN Address1
WHEN 'RecordId' THEN Cast(RecordId as varchar(10))
WHEN 'EliteStatus' THEN Cast(EliteStatus as varchar(10))
END
La seule chose que je n’ai pas corrigée est la sélection de CachedVenueAttributes qui semble construire une instruction where dans une boucle. Je pense que je pourrais mettre cela dans une fonction table, et le refactoriser de manière isolée par rapport au reste de la procédure.
J'aime le SQL dynamique pour les recherches.
Où je l'ai utilisé dans le passé, j'ai utilisé des instructions préparées .Net avec toute chaîne générée par l'utilisateur transmise en tant que paramètre NON inclus en tant que texte dans le code SQL.
Pour utiliser la solution existante, vous pouvez effectuer plusieurs actions pour réduire les risques.
- Entrée dans la liste blanche, validez-la de sorte qu'elle ne puisse contenir que a-zA-Z0-9 \ w (chiffres alpha et espaces blancs) (valeur incorrecte si vous devez prendre en charge les caractères Unicode)
- Exécute n’importe quel SQL dynamique en tant qu’utilisateur restreint. Définissez le propriétaire du proc stocké sur un utilisateur qui n’a qu’un accès en lecture aux tables concernées. refuser l'écriture à toutes les tables ect. De même, lorsque vous appelez ce processus stocké, vous devrez peut-être le faire avec un utilisateur avec des restrictions similaires sur ce qu'il est en mesure de faire, car MS-SQL exécute le SQL dynamique dans une procédure stockée en tant qu'utilisateur appelant et non le propriétaire du processus stocké.
Je me suis rendu compte que c’était un très vieux billet, mais lorsqu’on fait des choses comme:
AND
(
(@showAll = 1)
OR (@showAll <> 1
AND dbo.GeocodeDistanceMiles(Latitude,Longitude,@latitude,@longitude) <= convert(varchar,@range))
)
... une OPTION (RECOMPILE)
aidera généralement à choisir un plan plus concis, tant qu'il ne sera pas exécuté mille fois par seconde ou quoi que ce soit.