Just to enlighten myself on the dangers, if I have something like
class User < AR Model Stuff
def authenticate(user, pass)
User.find_by_sql("select * from Users where user = #{user} and
password = #{pass}")
end
end
What could I give that function that would totally expose the DB?
Or, say I have a search box that doesn''t do any filtering.
class Article < AR Model Stuff
def search(query)
Articles.find_all(:conditions => "title = #{query}")
end
end
How could that be exploited?
SQL injection there is a bigger risk, I''d think. what if user is == "''foo''; drop table user;" On 6/1/05, Joe Van Dyk <joevandyk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> Just to enlighten myself on the dangers, if I have something like > > class User < AR Model Stuff > def authenticate(user, pass) > User.find_by_sql("select * from Users where user = #{user} and > password = #{pass}") > end > end > > > What could I give that function that would totally expose the DB? > > Or, say I have a search box that doesn''t do any filtering. > > class Article < AR Model Stuff > def search(query) > Articles.find_all(:conditions => "title = #{query}") > end > end > > How could that be exploited? > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >
On 6/1/05, Michael Campbell <michael.campbell-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> SQL injection there is a bigger risk, I''d think. > > what if user is == "''foo''; drop table user;"Oh whoops, yes, SQL injection is what I meant. So generally, you''d just add a semi-colon and another statement? That wouldn''t be a valid SQL statement, would it?> On 6/1/05, Joe Van Dyk <joevandyk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > Just to enlighten myself on the dangers, if I have something like > > > > class User < AR Model Stuff > > def authenticate(user, pass) > > User.find_by_sql("select * from Users where user = #{user} and > > password = #{pass}") > > end > > end > > > > > > What could I give that function that would totally expose the DB? > > > > Or, say I have a search box that doesn''t do any filtering. > > > > class Article < AR Model Stuff > > def search(query) > > Articles.find_all(:conditions => "title = #{query}") > > end > > end > > > > How could that be exploited? > > _______________________________________________ > > Rails mailing list > > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > > http://lists.rubyonrails.org/mailman/listinfo/rails > > >
> Just to enlighten myself on the dangers, if I have something like > > class User < AR Model Stuff > def authenticate(user, pass) > User.find_by_sql("select * from Users where user = #{user} and > password = #{pass}") > end > end > > > What could I give that function that would totally expose the DB? > > Or, say I have a search box that doesn''t do any filtering. > > class Article < AR Model Stuff > def search(query) > Articles.find_all(:conditions => "title = #{query}") > end > end > > How could that be exploited?I would highly recommend that you read http://www.unixwiz.net/techtips/sql-injection.html. I didn''t understand SQL injection until I read that article. Essentially, when you don''t sanitize the input that is directly fed into a MySQL statement, you allow users to do as Michael showed ... insert extra queries. In his case, you lose all your data. It is also possible for the user to read most any other information in the table, including user names, and passwords ... so if the tables store any user information in plain text, that users personal info has been compromized... and if their plain-text password is the same as their password for their computer, you''ve now jeopordize more than just your webapp.
On Jun 1, 2005, at 9:01 AM, Joe Van Dyk wrote:> On 6/1/05, Michael Campbell <michael.campbell-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> SQL injection there is a bigger risk, I''d think. >> >> what if user is == "''foo''; drop table user;" >> > > Oh whoops, yes, SQL injection is what I meant. > > So generally, you''d just add a semi-colon and another statement? That > wouldn''t be a valid SQL statement, would it? >Well, it would be two valid SQL statements. Robert _______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
> So generally, you''d just add a semi-colon and another statement? That > wouldn''t be a valid SQL statement, would it?Yes. You can chain together MySQL queries/statements using a semicolon.
But then (in the above example) you''d have select * from Users where user = "foo"; drop table User; and password = "whatever" I guess the first statement would be executed, and the second one would fail? On 6/1/05, Belorion <belorion-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> > So generally, you''d just add a semi-colon and another statement? That > > wouldn''t be a valid SQL statement, would it? > > Yes. You can chain together MySQL queries/statements using a semicolon. >
Depends on the underlying db for the /precise/ syntax, but I think semicolon is generally a valid delimiter. On 6/1/05, Joe Van Dyk <joevandyk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> On 6/1/05, Michael Campbell <michael.campbell-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > SQL injection there is a bigger risk, I''d think. > > > > what if user is == "''foo''; drop table user;" > > Oh whoops, yes, SQL injection is what I meant. > > So generally, you''d just add a semi-colon and another statement? That > wouldn''t be a valid SQL statement, would it? > > > > On 6/1/05, Joe Van Dyk <joevandyk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > > Just to enlighten myself on the dangers, if I have something like > > > > > > class User < AR Model Stuff > > > def authenticate(user, pass) > > > User.find_by_sql("select * from Users where user = #{user} and > > > password = #{pass}") > > > end > > > end > > > > > > > > > What could I give that function that would totally expose the DB? > > > > > > Or, say I have a search box that doesn''t do any filtering. > > > > > > class Article < AR Model Stuff > > > def search(query) > > > Articles.find_all(:conditions => "title = #{query}") > > > end > > > end > > > > > > How could that be exploited? > > > _______________________________________________ > > > Rails mailing list > > > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > > > http://lists.rubyonrails.org/mailman/listinfo/rails > > > > > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >
Never use straight sql like this. Instead of a string you should
supply an array to AR. First part of the array is a string with
variables substituted as question-marks, after that the variables
which should take the location of the question marks.
Example:
def self.authenticate(login, pass)
find(:first, :conditions => ["login = ? AND password = ?",
login, pass])
end
def search(query)
Articles.find(:all, :conditions => ["title = ?",query] )
end
You are totally save from SQL injections this way as AR will sanitize
all parameters before adding them to the string ( or even better, use
the database native support for prepared statements if available ).
I highly recommend to check out the login generator ( gem install
login_generator ) which will set you up with a secure login system.
On 6/1/05, Joe Van Dyk <joevandyk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:> Just to enlighten myself on the dangers, if I have something like
>
> class User < AR Model Stuff
> def authenticate(user, pass)
> User.find_by_sql("select * from Users where user = #{user} and
> password = #{pass}")
> end
> end
>
>
> What could I give that function that would totally expose the DB?
>
> Or, say I have a search box that doesn''t do any filtering.
>
> class Article < AR Model Stuff
> def search(query)
> Articles.find_all(:conditions => "title = #{query}")
> end
> end
>
> How could that be exploited?
> _______________________________________________
> Rails mailing list
> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>
--
Tobi
http://www.snowdevil.ca - Snowboards that don''t suck
http://typo.leetsoft.com - Open source weblog engine
http://blog.leetsoft.com - Technical weblog
Joe Van Dyk wrote:> Just to enlighten myself on the dangers, if I have something like > > class User < AR Model Stuff > def authenticate(user, pass) > User.find_by_sql("select * from Users where user = #{user} and > password = #{pass}") > end > endYou forgot the single quotes there -- this way the SQL ought to give you trouble even for ''clean'' user names. I''ll assume that you meant this: User.find_by_sql("SELECT * FROM users WHERE user = ''#{user}'' AND password = ''#{pass}''") In this case I could just supply somebody else''s username (perhaps an admin one if I really want to be evil) and supply a crafted string like "foo'' OR ''a''=''a" as the password. You would then execute this query: SELECT * FROM users WHERE user = ''admin_guy'' AND password = ''foo'' OR ''a''=''a'' Which would probably log me in as the admin guy. There''s more stuff that can happen depending on the database backend like all your data getting stolen, deleted, bogus data getting entered, data getting manipulated, denial of service attacks and so on. You generally want to avoid all that by doing this stuff the Rails way: User.find_by_user_and_password(user, pass)
> There''s more stuff that can happen depending on the database backend > like all your data getting stolen, deleted, bogus data getting entered, > data getting manipulated, denial of service attacks and so on. You > generally want to avoid all that by doing this stuff the Rails way: > > User.find_by_user_and_password(user, pass)or the other rails way: User.find_by_sql(["SELECT * FROM users WHERE user = ? AND password = ?", user, pass]) -- Cheers Koz
This is a very clear reason why a single CRUD user is just a horrible idea. If the DB user under which this auth query is run is a user with only select privs on the user table and the user password table, the worst damage that could be done would be to inject a query to select all passwords and user ids. The display can be limited somewhat by your result set handlers, and it''s still a security problem, but it doesn''t allow privilege escalation, data destruction or modification, or anything that is an active threat to your data. Now, if only one user has access to the password table, and that user is used only for account creation and authentication -- not for the hundreds of other queries you might have in your app -- you eliminate the possibility of SQL injection on normal read operations from being able to access passwords. This should apply to content reads, too. All non-editing operations should be done with a SELECT-only user with SELECT access only to pure, readable content tables/columns/rows. This weekend, I''m going to try to find time to explore suggestions that arose yesterday by David wherein a DB connection might be switchable via model classes. I can''t find the original thread (perhaps it''s from a time before my joining the list) but SQL injection isn''t XSS. -tobyjoe On Jun 1, 2005, at 12:15 PM, Joe Van Dyk wrote:> But then (in the above example) you''d have > > select * from Users where user = "foo"; drop table User; and > password = "whatever" > > I guess the first statement would be executed, and the second one > would fail? > > > > On 6/1/05, Belorion <belorion-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >>> So generally, you''d just add a semi-colon and another statement? >>> That >>> wouldn''t be a valid SQL statement, would it? >>> >> >> Yes. You can chain together MySQL queries/statements using a >> semicolon. >> >> > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >
If you guys are interested in XSS attacks and the impact of these attacks I would recommend going to see Jeremiah Grossman''s presentation at BlackHat next week. I think it will finally open everyones eyes to just how bad these issues can be. Full disclosure: Jeremiah and I work for the same company. On Jul 22, 2005, at 5:46 AM, Toby Boudreaux wrote:> This is a very clear reason why a single CRUD user is just a > horrible idea. > If the DB user under which this auth query is run is a user with > only select privs on the user table and the user password table, > the worst damage that could be done would be to inject a query to > select all passwords and user ids. The display can be limited > somewhat by your result set handlers, and it''s still a security > problem, but it doesn''t allow privilege escalation, data > destruction or modification, or anything that is an active threat > to your data. > > Now, if only one user has access to the password table, and that > user is used only for account creation and authentication -- not > for the hundreds of other queries you might have in your app -- you > eliminate the possibility of SQL injection on normal read > operations from being able to access passwords. > > This should apply to content reads, too. All non-editing operations > should be done with a SELECT-only user with SELECT access only to > pure, readable content tables/columns/rows. > > This weekend, I''m going to try to find time to explore suggestions > that arose yesterday by David wherein a DB connection might be > switchable via model classes. > > I can''t find the original thread (perhaps it''s from a time before > my joining the list) but SQL injection isn''t XSS. > > -tobyjoe > > > On Jun 1, 2005, at 12:15 PM, Joe Van Dyk wrote: > > >> But then (in the above example) you''d have >> >> select * from Users where user = "foo"; drop table User; and >> password = "whatever" >> >> I guess the first statement would be executed, and the second one >> would fail? >> >> >> >> On 6/1/05, Belorion <belorion-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> >> >>>> So generally, you''d just add a semi-colon and another >>>> statement? That >>>> wouldn''t be a valid SQL statement, would it? >>>> >>>> >>> >>> Yes. You can chain together MySQL queries/statements using a >>> semicolon. >>> >>> >>> >> _______________________________________________ >> Rails mailing list >> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org >> http://lists.rubyonrails.org/mailman/listinfo/rails >> >> > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >- Bill
I''d love to, but it''s in Vegas, yeah? If only it were in NYC :) I try to stress the importance of security to folks, but very often people don''t want the responsibility/extra work. I think that, were Rails a touch more security-friendly, it would be a massive benefit. On Jul 22, 2005, at 1:09 PM, Bill Pennington wrote:> If you guys are interested in XSS attacks and the impact of these > attacks I would recommend going to see Jeremiah Grossman''s > presentation at BlackHat next week. I think it will finally open > everyones eyes to just how bad these issues can be. > > Full disclosure: Jeremiah and I work for the same company. > > On Jul 22, 2005, at 5:46 AM, Toby Boudreaux wrote: > > >> This is a very clear reason why a single CRUD user is just a >> horrible idea. >> If the DB user under which this auth query is run is a user with >> only select privs on the user table and the user password table, >> the worst damage that could be done would be to inject a query to >> select all passwords and user ids. The display can be limited >> somewhat by your result set handlers, and it''s still a security >> problem, but it doesn''t allow privilege escalation, data >> destruction or modification, or anything that is an active threat >> to your data. >> >> Now, if only one user has access to the password table, and that >> user is used only for account creation and authentication -- not >> for the hundreds of other queries you might have in your app -- >> you eliminate the possibility of SQL injection on normal read >> operations from being able to access passwords. >> >> This should apply to content reads, too. All non-editing >> operations should be done with a SELECT-only user with SELECT >> access only to pure, readable content tables/columns/rows. >> >> This weekend, I''m going to try to find time to explore suggestions >> that arose yesterday by David wherein a DB connection might be >> switchable via model classes. >> >> I can''t find the original thread (perhaps it''s from a time before >> my joining the list) but SQL injection isn''t XSS. >> >> -tobyjoe >> >> >> On Jun 1, 2005, at 12:15 PM, Joe Van Dyk wrote: >> >> >> >>> But then (in the above example) you''d have >>> >>> select * from Users where user = "foo"; drop table User; and >>> password = "whatever" >>> >>> I guess the first statement would be executed, and the second one >>> would fail? >>> >>> >>> >>> On 6/1/05, Belorion <belorion-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >>> >>> >>> >>>>> So generally, you''d just add a semi-colon and another >>>>> statement? That >>>>> wouldn''t be a valid SQL statement, would it? >>>>> >>>>> >>>>> >>>> >>>> Yes. You can chain together MySQL queries/statements using a >>>> semicolon. >>>> >>>> >>>> >>>> >>> _______________________________________________ >>> Rails mailing list >>> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org >>> http://lists.rubyonrails.org/mailman/listinfo/rails >>> >>> >>> >> >> _______________________________________________ >> Rails mailing list >> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org >> http://lists.rubyonrails.org/mailman/listinfo/rails >> >> > > > > - Bill > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >
Hey, I''m on it! Hehe. I''m still sorta getting deeper into Ruby and into the RoR guts, and I''d hate to commit a patch that was hacky. This weekend I hope to have a few interesting things written. -tj On Jul 22, 2005, at 1:21 PM, Sean T Allen wrote:> Toby Boudreaux wrote: > > >> I''d love to, but it''s in Vegas, yeah? If only it were in NYC :) >> >> > +1 > > >> I try to stress the importance of security to folks, but very >> often people don''t want the responsibility/extra work. I think >> that, were Rails a touch more security-friendly, it would be a >> massive benefit. >> >> > Didnt I see a posting about someone working on have more than just > a single CRUD user? > > > <sean.vcf> > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >
Toby Boudreaux wrote:> I''d love to, but it''s in Vegas, yeah? If only it were in NYC :) >+1> I try to stress the importance of security to folks, but very often > people don''t want the responsibility/extra work. I think that, were > Rails a touch more security-friendly, it would be a massive benefit. >Didnt I see a posting about someone working on have more than just a single CRUD user? _______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
Looking forward to seeing what you come up with... _______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
They will have the preso archived after the talk. You will miss out on demos but the content will be available. I will see if he would be interested in doing a webinar or something if there was enough interest. On Jul 22, 2005, at 10:13 AM, Toby Boudreaux wrote:> I''d love to, but it''s in Vegas, yeah? If only it were in NYC :) > > I try to stress the importance of security to folks, but very often > people don''t want the responsibility/extra work. I think that, were > Rails a touch more security-friendly, it would be a massive benefit. > > On Jul 22, 2005, at 1:09 PM, Bill Pennington wrote: > > >> If you guys are interested in XSS attacks and the impact of these >> attacks I would recommend going to see Jeremiah Grossman''s >> presentation at BlackHat next week. I think it will finally open >> everyones eyes to just how bad these issues can be. >> >> Full disclosure: Jeremiah and I work for the same company. >> >> On Jul 22, 2005, at 5:46 AM, Toby Boudreaux wrote: >> >> >> >>> This is a very clear reason why a single CRUD user is just a >>> horrible idea. >>> If the DB user under which this auth query is run is a user with >>> only select privs on the user table and the user password table, >>> the worst damage that could be done would be to inject a query to >>> select all passwords and user ids. The display can be limited >>> somewhat by your result set handlers, and it''s still a security >>> problem, but it doesn''t allow privilege escalation, data >>> destruction or modification, or anything that is an active threat >>> to your data. >>> >>> Now, if only one user has access to the password table, and that >>> user is used only for account creation and authentication -- not >>> for the hundreds of other queries you might have in your app -- >>> you eliminate the possibility of SQL injection on normal read >>> operations from being able to access passwords. >>> >>> This should apply to content reads, too. All non-editing >>> operations should be done with a SELECT-only user with SELECT >>> access only to pure, readable content tables/columns/rows. >>> >>> This weekend, I''m going to try to find time to explore >>> suggestions that arose yesterday by David wherein a DB connection >>> might be switchable via model classes. >>> >>> I can''t find the original thread (perhaps it''s from a time before >>> my joining the list) but SQL injection isn''t XSS. >>> >>> -tobyjoe >>> >>> >>> On Jun 1, 2005, at 12:15 PM, Joe Van Dyk wrote: >>> >>> >>> >>> >>>> But then (in the above example) you''d have >>>> >>>> select * from Users where user = "foo"; drop table User; and >>>> password = "whatever" >>>> >>>> I guess the first statement would be executed, and the second >>>> one would fail? >>>> >>>> >>>> >>>> On 6/1/05, Belorion <belorion-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >>>> >>>> >>>> >>>> >>>>>> So generally, you''d just add a semi-colon and another >>>>>> statement? That >>>>>> wouldn''t be a valid SQL statement, would it? >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> Yes. You can chain together MySQL queries/statements using a >>>>> semicolon. >>>>> >>>>> >>>>> >>>>> >>>>> >>>> _______________________________________________ >>>> Rails mailing list >>>> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org >>>> http://lists.rubyonrails.org/mailman/listinfo/rails >>>> >>>> >>>> >>>> >>> >>> _______________________________________________ >>> Rails mailing list >>> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org >>> http://lists.rubyonrails.org/mailman/listinfo/rails >>> >>> >>> >> >> >> >> - Bill >> >> _______________________________________________ >> Rails mailing list >> Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org >> http://lists.rubyonrails.org/mailman/listinfo/rails >> >> > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >- Bill
* Bill Pennington [2005-07-22 14:16]:> They will have the preso archived after the talk. You will miss out > on demos but the content will be available. >Assuming Vegas itself doesn''t just melt through the desert into the earth''s core before then. Isn''t a summer meeting in LV like a winter meeting in northern Minnesota? -- ________________________________ toddgrimason*todd[ at ]slack.net
Pretty much but it is not my show ;-) Basically the guy that runs it gets cheap rates and no one ever leaves Caesars except to go home. Plus the bonus is Defcon is over the weekend which is a pretty fun party if you are into that sort of thing. On Jul 22, 2005, at 12:25 PM, Todd Grimason wrote:> > Assuming Vegas itself doesn''t just melt through the desert into the > earth''s core before then. Isn''t a summer meeting in LV like a winter > meeting in northern Minnesota? > > -- > > ________________________________ > toddgrimason*todd[ at ]slack.net > > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >- Bill