Paul E. G. Lynch
2013-Sep-11 22:36 UTC
Should "sanitize" return an empty string for non-strings?
If, in your view, you are expecting params[:name] to be a string, but
actually rails has parsed it into {"."=>"1234"} (or
something more
malicious), then currently
<%= sanitize(params[:name]) %> blows up because the hash does not respond
the expected methods from the sanitize call.
I could put in code to check that the params values I am sanitizing are
strings, but it seems like it would be better for sanitize to handle that,
and perhaps just return the empty string if the processing of the input
raises an exception.
--Paul
--
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web visit
https://groups.google.com/d/msgid/rubyonrails-talk/71ca60c3-40da-49a0-805b-648bab2b8d0b%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Robert Walker
2013-Sep-11 23:21 UTC
Re: Should "sanitize" return an empty string for non-strings?
Paul Lynch wrote in post #1121214:> If, in your view, you are expecting params[:name] to be a string, but > actually rails has parsed it into {"."=>"1234"} (or something more > malicious), then currently > <%= sanitize(params[:name]) %> blows up because the hash does not > respond > the expected methods from the sanitize call. > > I could put in code to check that the params values I am sanitizing are > strings, but it seems like it would be better for sanitize to handle > that, > and perhaps just return the empty string if the processing of the input > raises an exception.Hum. It seems to me that "blowing up" is the right thing to do in this scenario. More precisely an exception should be raised indicating a programmer mistake of passing an illegal argument to a method expecting a string. -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/c54d51850e1948568b77874beb9f21e1%40ruby-forum.com. For more options, visit https://groups.google.com/groups/opt_out.
Paul Lynch
2013-Sep-12 14:26 UTC
Re: Re: Should "sanitize" return an empty string for non-strings?
In this case it is user (hacker, scanner, etc.), not the programmer, who has passed the illegal argument. I don''t think that should result in a 500 server error. To avoid that, either the programmer has to check each input parameter to make sure it is a string, or something like sanitize has to make the parameter safe. On Wed, Sep 11, 2013 at 7:21 PM, Robert Walker <lists-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote:> Paul Lynch wrote in post #1121214: > > If, in your view, you are expecting params[:name] to be a string, but > > actually rails has parsed it into {"."=>"1234"} (or something more > > malicious), then currently > > <%= sanitize(params[:name]) %> blows up because the hash does not > > respond > > the expected methods from the sanitize call. > > > > I could put in code to check that the params values I am sanitizing are > > strings, but it seems like it would be better for sanitize to handle > > that, > > and perhaps just return the empty string if the processing of the input > > raises an exception. > > Hum. It seems to me that "blowing up" is the right thing to do in this > scenario. More precisely an exception should be raised indicating a > programmer mistake of passing an illegal argument to a method expecting > a string. > > -- > Posted via http://www.ruby-forum.com/. > > -- > You received this message because you are subscribed to a topic in the > Google Groups "Ruby on Rails: Talk" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/rubyonrails-talk/6P_vm57_km8/unsubscribe > . > To unsubscribe from this group and all its topics, send an email to > rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org > To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org > To view this discussion on the web visit > https://groups.google.com/d/msgid/rubyonrails-talk/c54d51850e1948568b77874beb9f21e1%40ruby-forum.com > . > For more options, visit https://groups.google.com/groups/opt_out. >-- Paul Lynch National Library of Medicine -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAP3VaCUVhAAeEqHURZGEoZaXotbfGFw3Xb%3DJ1z63fw%2B8%2BM%2Bk2w%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.
Jordon Bedwell
2013-Sep-12 14:31 UTC
Re: Re: Should "sanitize" return an empty string for non-strings?
On Thu, Sep 12, 2013 at 9:26 AM, Paul Lynch <plynchnlm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> In this case it is user (hacker, scanner, etc.), not the programmer, who has > passed the illegal argument. I don''t think that should result in a 500 > server error. To avoid that, either the programmer has to check each input > parameter to make sure it is a string, or something like sanitize has to > make the parameter safe.It''s your problem and even more so *your* job to enforce types, not sanitizes problem to enforce your type. Sanitize is a helper, it is not part of your normal routing therefore it should not have to allow you be more oblivious as to what is going on in your software and how to handle and think about intentional/misintentional malicious users. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAM5XQny571Ov5F4_Jw4HQLkgG%3Dt4Y%3DW3CbjVBoc-Zoa6k8Vixw%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.
Matt Jones
2013-Sep-12 14:38 UTC
Re: Re: Should "sanitize" return an empty string for non-strings?
It''s unclear to me why you *wouldn''t* want a 500 ISE here. Silently swallowing ArgumentError or NoMethodError is a terrible idea, since it also can obscure real bugs. If you really want that behavior, try: <%= sanitize(params[:name]) rescue '''' %> --Matt Jones On Thursday, 12 September 2013 09:26:18 UTC-5, Paul E. G. Lynch wrote:> > In this case it is user (hacker, scanner, etc.), not the programmer, who > has passed the illegal argument. I don''t think that should result in a 500 > server error. To avoid that, either the programmer has to check each input > parameter to make sure it is a string, or something like sanitize has to > make the parameter safe. > > > > On Wed, Sep 11, 2013 at 7:21 PM, Robert Walker <li...-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org<javascript:> > > wrote: > >> Paul Lynch wrote in post #1121214: >> > If, in your view, you are expecting params[:name] to be a string, but >> > actually rails has parsed it into {"."=>"1234"} (or something more >> > malicious), then currently >> > <%= sanitize(params[:name]) %> blows up because the hash does not >> > respond >> > the expected methods from the sanitize call. >> > >> > I could put in code to check that the params values I am sanitizing are >> > strings, but it seems like it would be better for sanitize to handle >> > that, >> > and perhaps just return the empty string if the processing of the input >> > raises an exception. >> >> Hum. It seems to me that "blowing up" is the right thing to do in this >> scenario. More precisely an exception should be raised indicating a >> programmer mistake of passing an illegal argument to a method expecting >> a string. >> >> -- >> Posted via http://www.ruby-forum.com/. >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "Ruby on Rails: Talk" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/rubyonrails-talk/6P_vm57_km8/unsubscribe >> . >> To unsubscribe from this group and all its topics, send an email to >> rubyonrails-ta...-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org <javascript:>. >> To post to this group, send email to rubyonra...-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org<javascript:> >> . >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/rubyonrails-talk/c54d51850e1948568b77874beb9f21e1%40ruby-forum.com >> . >> For more options, visit https://groups.google.com/groups/opt_out. >> > > > > -- > Paul Lynch > National Library of Medicine >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/3042ea8d-7b0f-4080-9c95-1fe4202919ea%40googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
Paul Lynch
2013-Sep-12 17:22 UTC
Re: Re: Should "sanitize" return an empty string for non-strings?
It is because I am trying to distinguish between "real bugs" and bad input (a 400 error) that I don''t want this to be a 500 error. I have a rescue action that sends me an email when a 500 error occurs (though it limits the number it sends) because if there is a bug in the program that users are running into, I want to know about it. I don''t want to get those alerts every time someone intentionally sends bad input. It seems to me that the convenience of having sanitize just handle the exception outweighs the possibility of missing bugs involving sanitize calls (which seems slight to me, though maybe I am not thinking of some use case). Thanks for the suggestion of adding a rescue. I might just patch sanitize locally so I don''t have to type the rescue each time. On Thu, Sep 12, 2013 at 10:38 AM, Matt Jones <al2o3cr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> It''s unclear to me why you *wouldn''t* want a 500 ISE here. Silently > swallowing ArgumentError or NoMethodError is a terrible idea, since it also > can obscure real bugs. > > If you really want that behavior, try: > > <%= sanitize(params[:name]) rescue '''' %> > > --Matt Jones > > On Thursday, 12 September 2013 09:26:18 UTC-5, Paul E. G. Lynch wrote: > >> In this case it is user (hacker, scanner, etc.), not the programmer, who >> has passed the illegal argument. I don''t think that should result in a 500 >> server error. To avoid that, either the programmer has to check each input >> parameter to make sure it is a string, or something like sanitize has to >> make the parameter safe. >> >> >> >> On Wed, Sep 11, 2013 at 7:21 PM, Robert Walker <li...-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org>wrote: >> >>> Paul Lynch wrote in post #1121214: >>> > If, in your view, you are expecting params[:name] to be a string, but >>> > actually rails has parsed it into {"."=>"1234"} (or something more >>> > malicious), then currently >>> > <%= sanitize(params[:name]) %> blows up because the hash does not >>> > respond >>> > the expected methods from the sanitize call. >>> > >>> > I could put in code to check that the params values I am sanitizing are >>> > strings, but it seems like it would be better for sanitize to handle >>> > that, >>> > and perhaps just return the empty string if the processing of the input >>> > raises an exception. >>> >>> Hum. It seems to me that "blowing up" is the right thing to do in this >>> scenario. More precisely an exception should be raised indicating a >>> programmer mistake of passing an illegal argument to a method expecting >>> a string. >>> >>> -- >>> Posted via http://www.ruby-forum.com/. >>> >>> -- >>> You received this message because you are subscribed to a topic in the >>> Google Groups "Ruby on Rails: Talk" group. >>> To unsubscribe from this topic, visit https://groups.google.com/d/** >>> topic/rubyonrails-talk/6P_**vm57_km8/unsubscribe<https://groups.google.com/d/topic/rubyonrails-talk/6P_vm57_km8/unsubscribe> >>> . >>> To unsubscribe from this group and all its topics, send an email to >>> rubyonrails-ta...@**googlegroups.com. >>> To post to this group, send email to rubyonra...@googlegroups.**com. >>> To view this discussion on the web visit https://groups.google.com/d/** >>> msgid/rubyonrails-talk/**c54d51850e1948568b77874beb9f21** >>> e1%40ruby-forum.com<https://groups.google.com/d/msgid/rubyonrails-talk/c54d51850e1948568b77874beb9f21e1%40ruby-forum.com> >>> . >>> For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/opt_out> >>> . >>> >> >> >> >> -- >> Paul Lynch >> National Library of Medicine >> > -- > You received this message because you are subscribed to a topic in the > Google Groups "Ruby on Rails: Talk" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/rubyonrails-talk/6P_vm57_km8/unsubscribe > . > To unsubscribe from this group and all its topics, send an email to > rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org > To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org > To view this discussion on the web visit > https://groups.google.com/d/msgid/rubyonrails-talk/3042ea8d-7b0f-4080-9c95-1fe4202919ea%40googlegroups.com > . > > For more options, visit https://groups.google.com/groups/opt_out. >-- Paul Lynch National Library of Medicine -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAP3VaCVKJSaRE7jBeYEqKKDWE5VKu5C-9o0jqyoHppMX-CZYZA%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.
Jordon Bedwell
2013-Sep-12 17:27 UTC
Re: Re: Should "sanitize" return an empty string for non-strings?
On Thu, Sep 12, 2013 at 12:22 PM, Paul Lynch <plynchnlm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> It is because I am trying to distinguish between "real bugs" and bad input > (a 400 error) that I don''t want this to be a 500 error. I have a rescue > action that sends me an email when a 500 error occurs (though it limits the > number it sends) because if there is a bug in the program that users are > running into, I want to know about it. I don''t want to get those alerts > every time someone intentionally sends bad input. It seems to me that the > convenience of having sanitize just handle the exception outweighs the > possibility of missing bugs involving sanitize calls (which seems slight to > me, though maybe I am not thinking of some use case).His suggestion was not to rescue the error, it was an example of how you could. To rescue any error in an app is asking for edge cases unless they are specific errors for specific actions such as a RoutingError. The proper way to handle this is to type the input and return long before you even hit sanitize and have to rescue for: 1.) Better performance on errors an 2.) Better ability to know how your app is behaving. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAM5XQnxrAW%2Bii2wwV58LzV4nQps5K1G1-nSZG0KUUKnJAHL%2BxA%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.
Hassan Schroeder
2013-Sep-12 18:19 UTC
Re: Should "sanitize" return an empty string for non-strings?
On Wed, Sep 11, 2013 at 3:36 PM, Paul E. G. Lynch <plynchnlm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> If, in your view, you are expecting params[:name] to be a string, but > actually rails has parsed it into {"."=>"1234"} (or something more > malicious)Params are strings by definition; can you provide a test case/code that demonstrates where this is not the case? -- Hassan Schroeder ------------------------ hassan.schroeder-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org http://about.me/hassanschroeder twitter: @hassan -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CACmC4yCEuL7ftA_fn1SxsexHk4Wkm%2BOu4ycnVAdvsTx4Jio-cA%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.
Robert Walker
2013-Sep-12 18:52 UTC
Re: Should "sanitize" return an empty string for non-strings?
Hassan Schroeder wrote in post #1121316:> On Wed, Sep 11, 2013 at 3:36 PM, Paul E. G. Lynch <plynchnlm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > wrote: >> If, in your view, you are expecting params[:name] to be a string, but >> actually rails has parsed it into {"."=>"1234"} (or something more >> malicious) > > Params are strings by definition; can you provide a test case/code > that demonstrates where this is not the case?Not necessarily the case. For example the create and update actions in a users_controller will likely contain the user model in the params hash as a hash keyed by :user: params[:user] => { :first_name => "John", :last_name => "Doe", age: 25 } -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/5610708d401ca245037be4c44a5f58bb%40ruby-forum.com. For more options, visit https://groups.google.com/groups/opt_out.
Hassan Schroeder
2013-Sep-12 19:47 UTC
Re: Re: Should "sanitize" return an empty string for non-strings?
On Thu, Sep 12, 2013 at 11:52 AM, Robert Walker <lists-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote:>> Params are strings by definition; can you provide a test case/code >> that demonstrates where this is not the case? > > Not necessarily the case. For example the create and update actions in a > users_controller will likely contain the user model in the params hash...Meh, you''re right. I was thinking of the over-the-wire definition of HTTP parameter name/value pairs but yes, at the controller level Rails has already decided certain representations are more than that. Never mind! -- Hassan Schroeder ------------------------ hassan.schroeder-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org http://about.me/hassanschroeder twitter: @hassan -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CACmC4yA5%3DAyWG_iabSeQ3CQW5bwBjoOS%3DfRpEKB1j%3DvS2aD%3D%2BA%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.