Ist diese gespeicherte Prozedur vor SQL-Injektion geschützt?

Question

Dieser gespeicherte Proc führt sql mit Parametern unter Verwendung von sp_executesql aus.
Ist es sicher vor SQL-Injektion?

create procedure ExecutePeopleFilter 
    (@lastNameFilter varchar(20), 
    @companyNameFilter varchar(20), 
    @ageFilter int, 
    @dateFilter datetime) 
as 
begin 
    declare @sql varchar(4000) 
    declare @params varchar(1000) 
    declare @whereClause varchar(1000) 

    set @whereClause = '' 

    if ISNULL(@lastNameFilter,'') <> '' 
    begin 
     if (LEN(@whereClause) <> 0) set @whereClause += ' and ' 
     if (LEN(@lastNameFilter) < 20) set @lastNameFilter += '%' 
     set @whereClause += 'LastName like @lastName ' 
    end 

    if ISNULL(@companyNameFilter,'') <> '' 
    begin 
     if (LEN(@whereClause) <> 0) set @whereClause += ' and ' 
     if (LEN(@companyNameFilter) < 20) set @companyNameFilter += '%' 
     set @whereClause += 'CompanyName like @companyName ' 
    end 

    if @ageFilter is not null 
    begin 
     if (LEN(@whereClause) <> 0) set @whereClause += ' and '  
     set @whereClause += 'Age = @age ' 
    end 

    if @dateFilter is not null 
    begin 
     if (LEN(@whereClause) <> 0) set @whereClause += ' and ' 
     set @whereClause += 'StartDate = @startDate ' 
    end 


    set @sql = 'select FirstName, LastName, CompanyName, Age, StartDate 
     from People' 
    if (LEN(@whereClause) <> 0) set @sql += ' where ' + @whereClause 

    set @params = '@lastName varchar(20), 
     @companyName varchar(20), 
     @age int, 
     @startDate datetime' 

    execute sp_executesql @sql, @params, 
     @lastName = @lastNameFilter, 
     @companyName = @companyNameFilter, 
     @age = @ageFilter, 
     @startDate = @dateFilter 
end 

Answer 1

Ziemlich viel.

Der Schlüssel zur Verhinderung Injektion SQL ist korrekte Handhabung von Parametern über eine „approved“ -Mechanismus und String-Verkettung zu vermeiden.

Ihr Code nicht bauen einen String mit den Parametern auf: sie werden getrennt und über Sp_executesql gereinigt.

Ob Sie es auf diese Weise tun würde, ist eine andere Sache ... wie andere Antworten zeigen

Answer 2

Warum in der gespeicherten Prozedur? Eine bessere Lösung könnte sein, dies auf der Client-Seite anzugreifen, Strings zu entgehen und Längen zu prüfen, bevor der gespeicherte Proc aufgerufen wird. Bibliotheken wie die MS Enterprise DAAB (.NET) bieten bequeme Möglichkeiten, dies zu tun, indem Sie den Datentyp und die Länge der Parameter angeben, wenn Sie sie zum Befehlsobjekt hinzufügen.

Answer 3

Sie brauchen keine dynamische SQL optional zu machen WHERE-Klauseln ... nur verwenden:

WHERE ((@x IS NULL) OR (@x = ...)) AND ... 

auch schneller sein sollte, und kein Risiko für die überquellenden Streicher, Injektion oder was auch immer.

So:

CREATE PROCEDURE ExecutePeopleFilter 
    (
    @lastNameFilter varchar(20), 
    @companyNameFilter varchar(20), 
    @ageFilter int, 
    @dateFilter datetime 
) 
AS 
    BEGIN 
    SELECT FirstName, LastName, CompanyName, Age, StartDate 
     FROM People 
     WHERE (
      (ISNULL(@lastNameFilter, '') = '') 
      OR (LastName LIKE @lastNameFilter+'%') 
      ) 
     AND (
      (ISNULL(@companyNameFilter, '') = '') 
      OR (LastName LIKE @companyNameFilter+'%') 
      ) 
     AND (
      (@ageFilter IS NULL) 
      OR (Age = @ageFilter) 
      ) 
     AND (
      (@dateFilter IS NULL) 
      OR (StartDate = @dateFilter) 
      ) ; 
    END 

Answer 4

Sie nie neben bekannten etwas verketten, an die Datenbank ausgegeben hartcodierte Werte in eine SQL-Anweisung zu Daher ist es sicher vor allen derzeit bekannten SQL-Injection-Ansätzen (und sollte robust gegenüber zukünftigen Angriffen sein). Es hat jedoch andere Probleme (wie @StartDate wird nicht deklariert).

Answer 5

Ja, Ihre Verwendung von String concat und Sp_executesql ist richtig und SQL-Injektionen wird kein Problem sein.

Und nein, nur weil LINQ die neue Hotness ist, bedeutet nicht, es ist die richtige Lösung.

das gesagt ist, Ihre varchar (20) Parameter leicht Überlauf kann, können Sie diejenigen oben ein wenig, zumindest der Größe des tatsächlichen Feld stoßen wollen.

Answer 6

I denke so. Zumindest ist es sieht in Ordnung. Aber es ist wahrscheinlich nicht so, wie ich es machen würde.

Einer der wichtigsten Mieter der Sicherheit ist nie nie nie schreiben Sie Ihren eigenen Sicherheitscode. Sie immer wollen so viel wie möglich auf die Mechanismen von und in Ihre Plattform eingebaut lehnen. Sie tun das etwas über Ihre Verwendung von sp_executesql, aber ich nehme an, dass Sie auch mehr tun könnten.

---

Auf Wunsch würde ich es wahrscheinlich mehr wie folgt aus:

create procedure ExecutePeopleFilter 
    (@lastNameFilter varchar(20) = NULL, 
    @companyNameFilter varchar(20) = NULL, 
    @ageFilter int = NULL, 
    @dateFilter datetime = NULL) 
as 
begin 

    if (LEN(@lastNameFilter) < 20) set @lastNameFilter += '%' 
    if (LEN(@companyNameFilter) < 20) set @companyNameFilter += '%' 

    SELECT FirstName, LastName, CompanyName, Age, StartDate 
    FROM People 
    WHERE LastName LIKE COALESCE(@lastNameFilter, LastName) 
     AND CompanyName LIKE COALESCE(@companyNameFilter, CompanyName) 
     AND Age   = COALESCE(@ageFilter, Age) 
     AND StartDate = COALESCE(@dateFilter, StartDate) 

end 

Keine dynamische SQL benötigt, und keine "OR" s in der Klausel WHERE.

Answer 7

Auch wenn es ... "ugh" ist.

Warum nicht stattdessen eine temporäre Tabelle verwenden, sie mit den IDs der Elementergebnisse füllen, die mit Ihren nicht optionalen Parametern übereinstimmen, und dann den Rest aus der temporären Tabelle basierend auf den angegebenen Parametern entfernen? Sobald Sie dies getan haben, schließen Sie sich einfach dem Ergebnissatz an, nach dem Sie suchen.

CREATE TABLE #People 
    (personid int) 
INSERT INTO #People SELECT personid FROM people 
IF NOT @lastNameParam IS NULL 
    DELETE FROM #People WHERE personid NOT IN (SELECT personid FROM people WHERE lastname LIKE @lastNameParam + '%') 
-- And so on...