Queste iniezioni SQL verificano una cattiva idea?

3

Lettura Prevenzione degli attacchi SQL Injection nelle stored procedure ha iniziato questo dibattito tra me e il mio partner --- e cercare l'approccio giusto non è così semplice.

Non sono un esperto di sicurezza, ma cosa fa una persona quando dai un'occhiata alle prime righe di questa stored procedure ... e vedi questo.

Non abbiamo avuto problemi di sicurezza significativi, ma non voglio presumere che questo approccio funzioni ... Se è così, lascia perdere o migliora.

ASP:

Dim conStr = ConfigurationSettings.AppSettings("WebUser")
Dim command = ("Execute dbo.web_Insert_usr_Comments @Location," & _
                    "@userexp, @full_name, @emailAddr, @comments")

Using con As New SqlConnection(conStr)
    Using cmd As New SqlCommand(command, con)
        cmd.Parameters.AddWithValue("@Location", Request.QueryString("Location"))
        cmd.Parameters.AddWithValue("@userexp", Request.QueryString("usrexp"))
        cmd.Parameters.AddWithValue("@comments", Request.QueryString("comments"))
        cmd.Parameters.AddWithValue("@full_name", Request.QueryString("full_name"))
        cmd.Parameters.AddWithValue("@emailAddr", Request.QueryString("emailAddr"))

        con.Open()
        cmd.ExecuteNonQuery()
    End Using                           
End Using 

TSQL:

 ALTER PROCEDURE [dbo].[web_Insert_usr_Comments] 
        @Location varchar(255), 
        @usrexp int = NULL,
        @full_name varchar(255) = NULL,
        @emailAddr varchar(320) = NULL,
        @comments varchar(125) = NULL

    AS
    BEGIN
        DECLARE @securityChk varchar(255),  @haveContact INT

        SET @securityChk = (SELECT @location + CAST(@usrexp AS VARCHAR(2)) + @full_name + @emailAddr + @usr_comments )  

        IF @securityChk LIKE '%SELECT%' RETURN;
        IF @securityChk LIKE '%DROP%'   RETURN;
        IF @securityChk LIKE '%INSERT%' RETURN;
        IF @securityChk LIKE '%DELETE%' RETURN;
        IF @securityChk LIKE '%EXE%'    RETURN;


    SET @haveContact =   ( SELECT  (count(*))
                FROM dbo.contacts 
                WHERE ( @emailAddr = E_mail_address AND  @location = company))
    --> Check and see if we have this person contact info                
    IF ( @haveContact = 0 )
       BEGIN 
        INSERT INTO dbo.contacts(e_mail_address, company, nick_name)
        VALUES (@emailAddr, @location, @full_name)
       END
    ELSE
        BEGIN
          INSERT INTO web_comments_dtl(timeGMT, form_id, contact_id, comment, user_exp_ranking, IP_Addr)
          SELECT GETDATE()
           , 1 --> TODO: Need better logic for form_id  
           , ( SELECT MAX(id) 
               FROM dbo.contacts
               WHERE  ( @emailAddr = E_mail_address AND  @location = company)
            OR ( @emailAddr = E_mail_address AND @full_name  = nick_name )
            OR ( @location = company))
          , @RFXcomments
          , @usrexp
          , dbo.fnBinaryIPv4(@ip_Addr)
        END
    END

Cosa mi piacerebbe fare, ma non sono sicuro di quale sia l'opzione più efficiente.

  1. Dovrebbe trascorrere una settimana o due e indagare sui problemi di sicurezza (se ce n'è uno) e il modulo di input dell'utente che chiama questo SP
  2. Riscrivi tutto, incluso SP, codice dietro e riconfigura IIS
  3. Aggiungi altre clausole alla dichiarazione precedente.

Non penso che ci sia un problema di sicurezza ... ma se il ragazzo prima decide di inserire questo nel suo codice - cosa dovrebbe fare un giovane DBA.

    
posta AAH-Shoot 23.12.2013 - 18:10
fonte

4 risposte

7

Effettuare questo tipo di blacklist delle parole nei tuoi argomenti non è necessario con le stored procedure implementate correttamente e porterà solo a un'esperienza utente negativa nel caso in cui questo non funzioni su contenuti benigni.

Se si sta utilizzando una stored procedure e strutturando la query con i parametri che vengono passati alla stored procedure, non è necessario eseguire questi controlli.

Se, tuttavia, concatenate le stringhe e le passate in una chiamata EXEC, vi state aprendo a SQL injection. Il mio consiglio è di usare semplicemente le stored procedure per quello che devono fare e affidarsi a quello per la protezione.

(A parte - 25 caratteri per il nome completo e l'indirizzo e-mail mi sembrano un po 'troppo brevi.)

    
risposta data 23.12.2013 - 18:56
fonte
3

Come altri hanno sottolineato, i controlli di SQL injection in questa procedura sono una cattiva idea. Non riescono a proteggere dall'iniezione SQL e bloccano alcuni input legittimi.

Ma per il tuo punto:

[what is a young DBA supposed to do?]

Sollevare questo problema con un DBA anziano o il tuo manager. Esprimi le tue preoccupazioni riguardo alla progettazione di questa procedura e ad altre procedure simili.

Quindi suggerisci che i parametri di ricerca sarebbero una soluzione migliore (più semplice, più efficace, standard del settore, meno possibilità di bloccare erroneamente l'input legittimo). Suggerire che il team dovrebbe renderlo uno standard di codifica per utilizzare i parametri di query come mezzo per difendersi dall'iniezione SQL.

Per i casi di contenuto dinamico che non può essere inserito come parametro (ad esempio, un nome di colonna dinamico), utilizza whitelisting invece di blacklisting .

La ragione per cui dico che è necessario portare questo ai manager o ai lead del team è che le procedure di riscrittura e la riconfigurazione di IIS interromperanno il loro programma corrente per portare a termine il lavoro. In qualità di membro junior del team, potresti non essere a conoscenza di altri vincoli o requisiti del progetto.

Le persone responsabili della pianificazione del progetto prendono la decisione su come e quando riscrivere tutto. Se lo prendi su te stesso per iniziarlo, potresti creare problemi per loro. La comunicazione aperta è il miglior piano.

    
risposta data 23.12.2013 - 19:40
fonte
2

Il modo corretto per prevenire l'SQL injection è di non costruire mai query SQL manipolando le stringhe di input controllati dall'utente, ma semplicemente usando parametri associati.

Ciò è nella tua applicazione mai fai:

db.execute_sql("SELECT * FROM users WHERE user_name = '" + user_name + 
               "' AND password_hash = HASH('" + password + "');"
              )

dove user_name potrebbe essere una variabile con un valore come '; DROP TABLE users; -- (quindi la stringa che deve essere eseguita dal database ora è SELECT * FROM users WHERE user_name = ''; DROP TABLE users; -- che elimina la tabella degli utenti e ignora tutto dopo il simbolo del commento.

Invece fai qualcosa come

db.execute_sql_with_params("SELECT * FROM users WHERE user_name = '?'" + 
                           " AND password_hash = HASH('?');", [user_name, password]);

dove indipendentemente dai valori delle variabili [user_name, password] la struttura dell'istruzione SQL non cambierà mai - cercherà sempre un nome utente con il contenuto della variabile user_name .

Quindi sì, qualsiasi tipo di controllo per determinati termini (che può essere valido e non catturerà completamente tutti gli attacchi) è una cattiva idea.

Questo è leggermente diverso dal dire che solo le stored procedure usate - stored procedure - sono solo funzioni SQL memorizzate. Anche se sarebbe stupido, una stored procedure potrebbe creare una query SQL in modo non sicuro manipolando le stringhe di input dell'utente, proprio come è possibile farlo nel codice dell'applicazione.

    
risposta data 10.01.2014 - 14:48
fonte
0

Si dovrebbe più o meno gestire questo nel codice dell'applicazione piuttosto che nella stored procedure. Un utente potrebbe avere un'email come [email protected].

Prova a utilizzare query parametrizzate qui.

    
risposta data 23.12.2013 - 18:47
fonte

Leggi altre domande sui tag