¿Cuáles son los peligros del SQL dinámico y pueden evitarse?
-
06-07-2019 - |
Pregunta
Acabamos de recibir el siguiente código como solución para una consulta de búsqueda complicada en una nueva aplicación proporcionada por desarrolladores offshore. Soy escéptico sobre el uso de SQL dinámico porque podría cerrar la instrucción SQL usando '; y luego ejecute un desagradable que se realizará en la base de datos!
¿Alguna idea sobre cómo solucionar el ataque de inyección?
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)
Solución
Como comentario aparte, no usaría EXEC
; preferiría usar sp_executesql
. Vea este magnífico artículo, La maldición y las bendiciones de SQL dinámico , por la razón y otra información sobre usando sql dinámico.
Otros consejos
Aquí hay una versión optimizada de la consulta anterior que no utiliza SQL dinámico ...
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
Lo único que no he solucionado es la selección de CachedVenueAttributes que parece generar una instrucción where en un bucle. Creo que podría poner esto en una función con valores de tabla y refactorizarlo de forma aislada para el resto del procedimiento.
Me gusta el SQL dinámico para búsqueda.
Donde lo he usado en el pasado, he usado declaraciones preparadas .Net con cualquier cadena generada por el usuario que se pasa como un parámetro NO incluido como texto en el SQL.
Para ejecutar con la solución existente, puede hacer varias cosas para mitigar el riesgo.
- Entrada de la lista blanca, valide la entrada para que solo pueda contener a-zA-Z0-9 \ w (números alfabéticos y espacios en blanco) (incorrecto si necesita admitir caracteres unicode)
- Ejecuta cualquier sql dinámico como usuario restringido. Establezca el propietario del proceso almacenado en un usuario que solo tenga acceso de lectura a las tablas correspondientes. negar escribir a todas las tablas ect. Además, al llamar a este proceso almacenado, es posible que deba hacerlo con un usuario con restricciones similares sobre lo que pueden hacer, ya que parece que MS-SQL ejecuta sql dinámico dentro de un proceso almacenado, ya que el usuario llamante no es el propietario del procedimiento almacenado.
Me di cuenta de que esta es una publicación muy antigua, pero cuando hago cosas como:
AND
(
(@showAll = 1)
OR (@showAll <> 1
AND dbo.GeocodeDistanceMiles(Latitude,Longitude,@latitude,@longitude) <= convert(varchar,@range))
)
... una OPTION (RECOMPILE)
generalmente ayudará a elegir un plan más conciso, siempre que no se ejecute mil veces por segundo ni nada.