Chiave hash convertita automaticamenta da symbol in stringa

[carminem]

Chiedo scusa per l’oggetto non molto chiaro, ma non riesco a
sintetizzare meglio.

In un’applicazione Rails ho il codice seguente:

def index

params.merge!(:include => [ :appointment => :patient ])

@receipts = Receipt.find(:all, options_from_query_params(params))

respond_to do |wants|
  wants.html { }
  wants.js { render :text => @receipts.to_flexigrid_json(

:field_order => [ :receipt_nr, :created_at, “patient.fullname”, :totale,
:bollo, :note ] ) }
end

end

La riga che mi dà problemi è: params.merge!(:include => [
:appointment => :patient ])
Ho provato anche con: params[:include] = [ :appointment => :patient ] e
la cosa non cambia (e non vedo perchè dovrebbe, aggiungerei).

Se metto un “puts params.inspect” mi ritrovo il seguente risultato:

{“action”=>“index”, “controller”=>“receipts”,
“include”=>[{“appointment”=>:patient}]}

Ovvero :appointment è stato convertito in stringa pur essendo passato
come symbol.
Questa cosa manda a rotoli la successiva chiamata “Receipt.find(:all,
…” che fà uso del codice seguente:

def options_from_query_params(params = {})
return params if params.empty?
page = params[:page].blank? ? 1 : params[:page].to_i
limit = params[:rp].blank? ? 1 : params[:rp].to_i
offset = (page-1) * limit
sortname = params[:sortname].blank? ? “id” : params[:sortname]
sortorder = params[:sortorder].blank? ? “asc” : params[:sortorder]
ordering = sortname + " " + sortorder

# Da rivedere e ripulire
qfilter = params[:qfilter].blank? ? "" : params[:qfilter]
qtype = params[:qtype].blank? ? "" : params[:qtype] + " like ? "
query = params[:query].blank? ? "%" : "%" + params[:query] + "%"
qtype = qfilter.blank? ? qtype : qfilter + ( qtype.blank? ? "" : "

and " + qtype)
associations = params[:include].blank? ? [] : params[:include]
{ :include => associations, :limit => limit, :offset => offset,
:order => ordering, :conditions => [qtype, query] }
end

Magari è una cosa banale, ma non riesco a trovare l’errore.

Rails si limita a segnalare un:

NoMethodError in ReceiptsController#index
You have a nil object when you didn’t expect it!
The error occurred while evaluating nil.name

Imputando la colpa alla riga:

@receipts = Receipt.find(:all, options_from_query_params(params))

Cosa posso controllare?

Grazie in anticipo per il Vs aiuto

[carminem]

Carmine M. wrote:

[snip]

The error occurred while evaluating nil.name

Cosa posso controllare?

Nel codice che hai postato non ho visto chiamate al metodo #name. Boh,
io guarderei l’oggetto su cui chiami quel metodo tanto per cominciare,
visto che ti trovi un bel nil imprevisto… script/server --debugger
potrebbe esserti utile.

[carminem]

Carmine M. wrote:

Chiedo scusa per l’oggetto non molto chiaro, ma non riesco a
sintetizzare meglio.

L’oggetto è relativamente chiaro, ma avresti potuto accorciare il codice
di molto.
Tutti i nomi di parametri vengono convertiti in stringa, hash o meno non
è rilevante. Questo secondo me è un bene; ad esempio in una GET HTTP
avere dei nomi di parametri che iniziano con un ‘:’ (o peggio, la sua
versione urlencoded) sarebbe decisamente strano.
Se a te serve un simbolo, sta a te convertirlo.

 ordering = sortname + " " + sortorder

Stai dicendo che lasci che l’utente specifichi direttamente i parametri
da passare a ActiveRecord e quindi a SQL?
… in bocca al lupo …

 associations = params[:include].blank? ? [] : params[:include]
 { :include =>  associations, :limit =>  limit, :offset =>  offset,

:order => ordering, :conditions => [qtype, query] }
end

Nel caso specifico (array di hash) puoi fare:

associations.map! do |a|
a.each_pair do |k,v|
{k.to_sym, v}
end
end

ciao,
Andrea

[carminem]

Andrea C. wrote:

Carmine M. wrote:
Tutti i nomi di parametri vengono convertiti in stringa, hash o meno non
è rilevante. Questo secondo me è un bene; ad esempio in una GET HTTP
avere dei nomi di parametri che iniziano con un ‘:’ (o peggio, la sua
versione urlencoded) sarebbe decisamente strano.
Se a te serve un simbolo, sta a te convertirlo.

 ordering = sortname + " " + sortorder

Stai dicendo che lasci che l’utente specifichi direttamente i parametri
da passare a ActiveRecord e quindi a SQL?
… in bocca al lupo …

Faccio una digressione su questo argomento perché è importante.
Il codice originale era

sortname = params[:sortname].blank? ? "id" : params[:sortname]
sortorder = params[:sortorder].blank? ? "asc" : params[:sortorder]
ordering = sortname + " " + sortorder

Immagina che come parametro sortorder ti venga passato (opportunamente
urlencodato) il valore “asc; drop table users;” e prova a ricostruire
l’SQL che verrà eseguito dalla tua applicazione. E questo è solo
l’ultimo dei problemi a cui ti stai esponendo. Ti possono creare
tabelle, cambiare password, creare stored procedure, trigger, di tutto.
Insomma possono ridurti il server ad un colabrodo, entrarci, prendere i
dati, sostituirli…

Googla SQL injection per maggiori dettagli e poi aggiungi la parola
Rails o ActiveRecord alle ricerche per vedere quali sono i modi standard
per evitare questo tipo di vulnerabilità in RoR.

Paolo

[carminem]

Mi autorispondo per dire che la cosa si è risolta con un:

@receipts = Receipt.find(:all, options_from_query_params.merge(:include
=> [{:appointment => :patient }]))

Ma, onestamente, non ho capito perchè.

Il fatto è che, alla fine della fiera, la find è una cosa del genere:

Receipt.find(:all, :include => [ “appointment” => :patient], :conditions
=> …

Ed ottengo l’errore di cui parlavo nel messaggio originale.
Se, invece del “appointment” appare il simbolo :appointment, tutto
funziona come previsto.

Il problema si manifesta anche dalla console di Rails.

Così funziona:
Receipt.find(:all, :include => [:appointment => :patient], :conditions
=> [‘patients.cognome like ?’,"%Mo%"])

Così, invece:

Receipt.find(:all, :include => [“appointment” => :patient], :conditions
=> [‘patients.cognome like ?’,"%Mo%"])

ottengo l’errore:

NoMethodError: You have a nil object when you didn’t expect it!
The error occurred while evaluating nil.macro

[carminem]

Andrea L. wrote:

Nel codice che hai postato non ho visto chiamate al metodo #name. Boh,
io guarderei l’oggetto su cui chiami quel metodo tanto per cominciare,
visto che ti trovi un bel nil imprevisto… script/server --debugger
potrebbe esserti utile.

Farò un giro con l’opzione “–debugger” per cercare di capirci di più.
In effetti, quel “nil.name” nel codice che gira non c’è proprio. Dalla
console di Rails, invece di “nil.name”, l’errore fà riferimento a
“nil.macro” come ho scritto nella mia risposta.

Grazie

[carminem]

Carmine M. wrote:

Personalmente, non mi cambia nulla tra l’avere le chiavi come simbolo o
come stringa. Solo che,
in questo caso (e non capisco perchè), se la chiave viene convertita in
stringa la “find” non funziona e Rails lamenta l’errore di cui ho
scritto.

Il mio “a te serve un simbolo” stava per: se devi chiamare un API che
richiede un simbolo.
Rails ti da un errore perché, come da documentazione di AR, serve un
simbolo e non una stringa.

Andrea

[carminem]

Andrea C. wrote:

Tutti i nomi di parametri vengono convertiti in stringa, hash o meno non
� rilevante. Questo secondo me � un bene; ad esempio in una GET HTTP
avere dei nomi di parametri che iniziano con un ‘:’ (o peggio, la sua
versione urlencoded) sarebbe decisamente strano.
Se a te serve un simbolo, sta a te convertirlo.

Personalmente, non mi cambia nulla tra l’avere le chiavi come simbolo o
come stringa. Solo che,
in questo caso (e non capisco perchè), se la chiave viene convertita in
stringa la “find” non funziona e Rails lamenta l’errore di cui ho
scritto.
Infatti, alla fine della fiera, la find che ottengo è del tipo:

Receipt.find(:all, :include => [“appointment” => :patient], :conditions
=> [‘patients.cognome like ?’,"%Mo%"])

La presenza di “appointment” al posto di :appointment crea i problemi.
Se, invece, scrivo una cosa
del genere:

@receipts = Receipt.find(:all, options_from_query_params.merge(:include
=> [{:appointment => :patient }]))

tutto funziona e :appointment viene mantenuto come simbolo.

Stai dicendo che lasci che l’utente specifichi direttamente i parametri
da passare a ActiveRecord e quindi a SQL?
… in bocca al lupo …

E’ una cosa terribilmente stupida e sono a conoscenza dei pericoli
dell’SQL Injection.
Il fatto è che, per ora almeno, la view fà uso di JS e del componente
flexigrid che compone l’elenco dei
parametri da passare per effettuare ricerche, paging e quant’altro.

Quindi, ho poco controllo a livello di UI e dovrei procedere alla
bonifica nel metodo “options_from_query_params”.

Grazie anche a te per l’aiuto!

[carminem]

Paolo M. wrote:

Faccio una digressione su questo argomento perché è importante.
Il codice originale era

sortname = params[:sortname].blank? ? "id" : params[:sortname]
sortorder = params[:sortorder].blank? ? "asc" : params[:sortorder]
ordering = sortname + " " + sortorder

Immagina che come parametro sortorder ti venga passato (opportunamente
urlencodato) il valore “asc; drop table users;” e prova a ricostruire
l’SQL che verrà eseguito dalla tua applicazione. E questo è solo
l’ultimo dei problemi a cui ti stai esponendo. Ti possono creare
tabelle, cambiare password, creare stored procedure, trigger, di tutto.
Insomma possono ridurti il server ad un colabrodo, entrarci, prendere i
dati, sostituirli…

Si, Paolo, come ho già detto, conosco il problema della SQL injection e
proprio nel metodo “options_from_query_params” mi sono preposto di fare
i dovuti controlli, dato che i parametri vengono composti dal componente
flexigrid (JS) su cui non ho molto controllo.

Googla SQL injection per maggiori dettagli e poi aggiungi la parola
Rails o ActiveRecord alle ricerche per vedere quali sono i modi standard
per evitare questo tipo di vulnerabilità in RoR.

Seguirò il tuo consiglio, c’è sempre da imparare.

Grazie

[carminem]

Paolo M. wrote:

Faccio una digressione su questo argomento perché è importante.
Il codice originale era

sortname = params[:sortname].blank? ? "id" : params[:sortname]
sortorder = params[:sortorder].blank? ? "asc" : params[:sortorder]
ordering = sortname + " " + sortorder

Immagina che come parametro sortorder ti venga passato (opportunamente
urlencodato) il valore “asc; drop table users;” e prova a ricostruire
l’SQL che verrà eseguito dalla tua applicazione. E questo è solo
l’ultimo dei problemi a cui ti stai esponendo.

Ciao Paolo,
ho provato ad inserire una query a caso per simulare sql injection in
una mia applicazione, prima manipolando i dati di un form, poi vedendo
che non avevo riscontri positivi direttamente dalla console, scrivendo
addirittura cose tipo

db_killer = Friend.find_by_sql(“select * from friends; drop database
friends_development;”)

ma ottengo solo errori ActiveRecord::StatementInvalid per la parte
successiva al punto e virgola, come se ActiveRecord non permettesse più
di una query per volta. Insomma, non sono riuscito a simulare una sql
injection del tipo da te segnalato… dove sbaglio?

Inoltre, anche se ci fosse effettivamente un problema di sql injection
credo che questo sia trascurabile se la query viene eseguita nella
sezione amministrativa di una applicazione da un utente amministratore,
in quanto questo avrebbe comunque tutte le possibilità per fare danni
in altri modi. Sarebbe un po’ come chiudere la finestra lasciando aperto
il portone.

Ciao
Luca

[carminem]

Nel caso ne avessi bisogno, ti segnalo questo link che tratta della
sicurezza nelle applicazioni Rails, (giusto un’infarinatura):

Rails Security Guide and Book - Ruby on Rails Security Project

Troverai qualche indicazione anche su SQL Injection.

Riguardo alla falla di sicurezza segnalata da Paolo posso dirti che cose
del tipo:

Receipt.find(:all, :include => [:appointment => :patient], :conditions
=> [‘patients.cognome like ?’,“%Mo%”], :order => “cognome ASC; drop
table users;”)

Pare vengano gestite piuttosto agevolmente da AR, al punto che la “drop
table” nella clausola order
viene allegramente ignorata.

Magari questo è un caso “banale” già contemplato da AR e quindi
opportunamente filtrato.
AR, a quanto ne sò, filtra da sè alcuni caratteri potenzialmente nocivi,
tipo '.
Questo è vero a patto di utilizzare, nelle :conditions ad esempio,
costrutti del tipo:

:conditions => [ ‘name = ? and surname like ?’, ‘nome’, ‘%cognome%’]

o, in alternativa, le notazioni:

:conditions => { :name => passed_name, :surname => passed_surname }

Se, invece, si fà uso della classica concatenazione di stringhe nella
:conditions, beh, allora c’è solo
da pregare.

Inoltre, anche se ci fosse effettivamente un problema di sql injection
credo che questo sia trascurabile se la query viene eseguita nella
sezione amministrativa di una applicazione da un utente amministratore,
in quanto questo avrebbe comunque tutte le possibilità per fare danni
in altri modi. Sarebbe un po’ come chiudere la finestra lasciando aperto
il portone.

Personalmente, non sono d’accordo su questa affermazione, in primis
perchè i privilegi
di amministratore potrebbero proprio essere ottenuti per mezzo di SQL
injection o XSS o chissÃ
quale altra diavoleria.
In seconda istanza per via della pericolosità delle abitudini.
Certo se, come nel mio caso, l’applicazione non è accessibile
dall’esterno, è poco più di un giocattolo
e l’utente è appena sceso dall’albero, forse, preoccuparsi di SQL
injection è esagerato

Saluti